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 <emman...@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 <sa...@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 <st...@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@lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > > _______________________________________________ > > hibernate-dev mailing list > > hibernate-dev@lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > _______________________________________________ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev