[hibernate-dev] Checkstyle and ORM

[Steve Ebersole]

Sure.  Personally I think the "trick" is to keep the number of failures
small.  In fact I initially listed just 9, but am already thinking of
moving at least one from high to medium (and therefore not considered a
failure).




On Mon, May 18, 2015 at 9:23 AM, Emmanuel Bernard <emmanuel at hibernate.org>
wrote:

> Note that custom checkstyle rule implementations does not fare well with
> IDE checkstyle rules enforcement.
> I can't make it reliably work on search and OGM.
>
> > On 16 mai 2015, at 16:11, Sanne Grinovero <sanne at hibernate.org> wrote:
> >
> > One nice thing of checkstyle is that it's easily extended, for example
> > in Search we added a couple of checks, like to ban double whitespaces:
> > -
> https://github.com/hibernate/hibernate-search/blob/master/build-config/src/main/java/org/hibernate/checkstyle/checks/regexp/DoubleSpacesCheck.java
> >
> > Or to ban specific import statements:
> > -
> https://github.com/hibernate/hibernate-search/blob/master/build-config/src/main/java/org/hibernate/checkstyle/checks/regexp/IllegalImport.java
> >
> > For example this is useful to iteratively move away from
> > commons-annotations, like we ban its AssertionFailure to favor usage
> > of a different AssertionFailure within Search:
> > -
> https://github.com/hibernate/hibernate-search/blob/master/build-config/src/main/resources/checkstyle.xml#L172
> >
> > But checkstyle isn't enough, I for one would love to ban auto-boxing
> > from our main code (it's ok in tests) as I'd rather the syntax call
> > explicit attention to it.
> > Sometimes it's dangerous for performance reasons, or because of other
> > subtle API compability reasons like the JBoss Logger upgrade.. but
> > checktyle can't do such things, you need to add additional tools to
> > the mix, like FindBugs.
> >
> > Have a look at the above linked checkstyle.xml for more ideas, but
> > sometimes I feel we've been too strict as it does fire back on quick
> > prototyping: it's very annoying to have your build fail for a couple
> > of style errors when you're deep in debugging some functionality..
> > I'd rather stick to it though, as it can be disabled locally with
> > appropriate flags and there finally are no more delays in pull request
> > reviews because of whitespacing.
> >
> > To make it slightly less annoying I've made a CheckStyle filter which
> > allows us to enforce some selected rules only on non-test code:
> > -
> https://github.com/hibernate/hibernate-search/blob/master/build-config/src/main/java/org/hibernate/checkstyle/filters/ExcludeTestPackages.java
> >
> > We could similarly extend it to enforce a package-info.java for all
> > non-test, non .impl packages?
> >
> > One which I would strongly enforce is about having the right copyright
> > headers. For example I've seen various hibernate-infinispan classes
> > using a non-Hibernate template. The traditional header includes the
> > year, which was sometimes problematic (like you'd copy an header in a
> > new class and mark it with a 6 years old copyright); so now we're
> > using a simplified copyright template which is shorter and has no
> > mention years:
> > -
> https://github.com/hibernate/hibernate-search/blob/master/engine/src/main/java/org/hibernate/search/analyzer/Discriminator.java#L1-L6
> > (according to legal that's good enough)
> >
> > -- Sanne
> >
> >
> >> On 16 May 2015 at 00:19, Steve Ebersole <steve at hibernate.org> wrote:
> >> FYI : https://hibernate.atlassian.net/browse/HHH-9803
> >>
> >> Its not super high on my priority list, but I want to get to a point
> where
> >> we can start to fail the build on "serious checkstyle regressions".
> Part
> >> of that is being more realistic with what is considered serious, and
> part
> >> of that is fixing up code.
> >>
> >> For example, we have quite a few warnings about spaces rather than tabs
> for
> >> indentation.  That's a serious one to me.  There are quite a few "unused
> >> import" warnings, again to me that's serious (it just looks sloppy).
> >>
> >> A few I have already disabled.  Checking that there is a
> package-info.java
> >> file for example.  I'd love to make a requirement that all API and SPI
> >> packages have one.  But as far as I know that detailing is not
> available in
> >> checkstyle, and I really dont overly care about package-info.java files
> for
> >> internal packages.
> >>
> >> These are just a few examples.  #909[1] (still running atm) is the first
> >> build with my preliminary work here.  Let me know if there are any
> checks
> >> anyone feels strongly about in one bucket or another.
> >>
> >>
> >> [1] http://ci.hibernate.org/job/hibernate-orm-master-h2/909/
> >> _______________________________________________
> >> hibernate-dev mailing list
> >> hibernate-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> > _______________________________________________
> > hibernate-dev mailing list
> > hibernate-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>