TeamNewPipe
Check code style into Git
We should check-in the code style into the repo - at least for IntelliJ.
This would help massively with keeping the code formatted correctly and we could maybe also remove the stupid checkstyle check that is ALWAYS executed when doing anything and only run it in the CI.
How can this be done? Checkout:
- https://github.com/litetex-oss/standard-maven-template/tree/e3e350f4c425c86c2fa71d49e66dac825fcdda69/.idea and
- https://github.com/litetex-oss/standard-maven-template/blob/e3e350f4c425c86c2fa71d49e66dac825fcdda69/.gitignore#L64-L82
In addition we could put the checkstyle check as a git push hook, provided it runs quickly enough. The Rust compiler repo has something like that, and I have to say it does help catching badly styled code without interfering with normal development.
git push hook,
I really don't like git push hooks because when I edit a Readme or update mocks I don't care about checkstyle rules.
I would just let it run in a dedicated CI job so it's clearly visible if Checkstyle failed or the build is broken ;)
I really don't like git push hooks because when I edit a Readme or update mocks I don't care about checkstyle rules.
It's quite easy to skip the hooks with --no-verify.
I would just let it run in a dedicated CI job so it's clearly visible if Checkstyle failed or the build is broken ;)
Sure, but the CI results would arrive a few minutes after having pushed, and by that time the PR opener might already have switched to doing something else. This would make reviews slower because you'd have to constantly tell the user to fix Checkstyle. While a git push hook would tell you immediately.
(this actually happened today to me: https://github.com/rust-lang/miri/pull/4452#issuecomment-3073038648 😬)
It's quite easy to skip the hooks with --no-verify.
Well could be problematic with people that don't use the CLI and a client such as GitHub desktop (so basically most developers)
Sure, but the CI results would arrive a few minutes after having pushed, and by that time the PR opener might already have switched to doing something else
Well they would have noticed this if they had the CheckStyle plugin running in their IDE :P Also running CheckStyle usually takes 30s-2min which is quite fast. The CI build currently alone takes 3minutes so running CheckStyle in parallel will likely do no harm.
This would make reviews slower because you'd have to constantly tell the user to fix Checkstyle.
I don't think so. Currently nearly everyone just straight up ignores the CI because it's useless and always red. (I'm allowed to say that because when I always come back after months everything is red and I fix it :P)
If we have a dedicated job that tells the commiter that only checkstyle is broken, the GitHub UI kind of automatically tells them what is going on and that they should fix this. Here's an example:
If the commiter still ignores this, the reviewers can also point that out and refuse to merge it.
Also IMHO: If one constantly forgets this and ignore the CI they should probably not be in a position to create a PR, that PR should be closed and the person ignored as they just steal our time.
We can also add a checkbox to the PR template that says:
- [ ] I checked that the Codestyle is applied correctly and the CI is not failing
or maybe automatically add post comment that says "Please fix your Codestyle" if it really escalates a lot