commons-dev mailing list archives

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

P.S.

Agree

P.P.S.

With

P.P.P.S.

you!

Gary

On Jun 23, 2017 10:28, "Simon Spero" <sesuncedu@gmail.com> wrote:

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

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