On Jun 23, 2017 11:03 AM, "Oliver Heger" <oliver.he...@oliver-heger.de> wrote:
However, letting the build fail because of checkstyle error is too restrictive IMHO. My approach is to work through the errors before creating a new release. This has the disadvantage that errors might accumulate; but from one release to the next one there is typically not that much. That's still my gut feeling- checkstyle build fails are infuriating - but annoyingly there are issues doing things at the release stage that are worse 😡 The big problem is related to recent discussions about code styles and commits, and the diff bloats and blame/praise shifting that happen when formatting drifts during a development cycle. Things turn out much better if formatting / style checking is done before a commit, or at least fails the build so that things can be fixed and disappeared if a PR is merged as a SquashBase (so a change + a revert cancels out ). IntelliJ has a checkstyle plugin, which, um, checks for checkstyle violations. This can run as a real time inspection, or during pre-commit analysis. It can also adjust code formatting options to match the checkstyle profile,which helps avoid many issues in the first place. Eclipse has similar support, but I am not an Eclipse user so I don't know the details. The validate phase checkstyle execution then becomes more of a backstop (validation is usually the right phase, since that is prior to compilation). It's a lot less annoying if it doesn't have too many "violations". Since checkstyle can be configured to allow a small number of violations, and to only treat rule-breakings above a certain severity as violations (error by default, but a pre-release execution could increase fussiness to include warnings). One thing that is tricky, but which can be worth it, is having a pseudo flag-day reformatting, where you use an ide to apply the code style to the entire project, then apply the changes all at once (possibly using a disposable committer, though this isn't as important if the annotate view you use shows a bit of commit message). It is a bit more effort if there a bunch of unmerged branches, but since the formatting changes are automated, they can be applied on the branch so it's not *that* horrible to get a mergeable branch back. History checking for a few lines may require going one revision back, but having all the format fixes in a single commit is a lot better than having them mixed in with real changes. Simon P. S. I prefer the builtin /google_checks.xml styleguide to the older sun_checks, partly because it's more up to date, and partly because Google provides native ide configuration files for Eclipse and intellij so there's no need to import the checkstyle file. The Checkstyle plugin tracks the Google style guide pretty closely, so as long as the plugin is up to date, there won't be much divergence. The most significant changes happen when there are new constructs to consider (e.g. lambda). P.P.S. I do think the google rules are wrong about horizontal alignment (at least for continuation lines). It makes a huge difference when you have to use obnoxious amounts of chained methods (can't say fluent without saying flu). P.P.S. I do like their tip on the appropriate use of finalize: *Tip:* Don't do it. If you absolutely must, first read and understand *Effective Java* Item 7, <http://books.google.com/books?isbn=8131726592> "Avoid Finalizers," very carefully, and *then* don't do it. I say finalizers are OK as long as they warn, and don't actually do anything ("you didn't commit a batch insert of 10M items. Were you sure? Yes / Tough").
