Re: [hibernate-dev] Checkstyle and ORM

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(a)hibernate.org&gt; 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/sr... > > Or to ban specific import statements: > - https://github.com/hibernate/hibernate-search/blob/master/build-config/sr... > > 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/sr... > > 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/sr... > > 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... > (according to legal that's good enough) > > -- Sanne > > >> On 16 May 2015 at 00:19, Steve Ebersole <steve(a)hibernate.org&gt; 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(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev