Should changes related to the setup and handling of checkstyle then be done for Commons as a whole?
Oliver (No P.S.) Am 23.06.2017 um 21:19 schrieb Gary Gregory: > I > > P.S. > > Agree > > P.P.S. > > With > > P.P.P.S. > > you! > > Gary > > On Jun 23, 2017 10:28, "Simon Spero" <sesunc...@gmail.com> wrote: > >> 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"). >> > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
