commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <>
Subject Re: [configuration] checkstyle fails build
Date Fri, 23 Jun 2017 19:19:42 GMT








On Jun 23, 2017 10:28, "Simon Spero" <> wrote:

> On Jun 23, 2017 11:03 AM, "Oliver Heger" <>
> 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, <> "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").

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message