commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Heger <>
Subject Re: [configuration] checkstyle fails build
Date Sat, 24 Jun 2017 16:31:39 GMT
Should changes related to the setup and handling of checkstyle then be
done for Commons as a whole?


(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" <> 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").

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message