Lots of Checkstyle changes to the IdP

Rod Widdowson rdw at steadingsoftware.com
Tue Sep 20 04:59:11 EDT 2016


> Okay if I do JPAR-60 on the IdP tomorrow afternoon/evening (Tue Sep 20) ?

Sounds like great news Tom.  Thanks ......  and no problems for me.

> it would be nice to get that code Checkstyle clean for 3.3.

I think that would be possible, but I would urge caution.  Remember being Checkstyle clean isn't an end in and of itself, it's just a means to the end - which is clear, maintainable, bug free code.  I'd sooner have a few Checkstyle warnings in 3.3 than have a nasty bug leap out and bite us.

It's very easy to get a module Checkstyle clean just by turning off the warnings.

I'll be the first to admit that I do that but it is always in well thought out situations:

 - When Checkstyle has changed the rules under our feet and things which were ok suddenly aren't (the multiple returns case).
      I'd like to revisit that rule at some stage but it feels that being closer to clean is pre-requisite

 - When essentially linear code breaks method length and/or cyclometric complexity rules.  The two canonical cases are the parsers and (I believe) SWF action beans' execute statements.  I started by trying to avoid this but the code became disgusting; so after discussion we went back to the longer code flows.  In this case Checkstyle clean-ness is opposed to our end.

Beyond that it becomes problematic, and mostly in the area of deprecation.

It's very easy (and often very wrong) to just supress the warning.

I thought long and hard before I dealt with a deprecated class (some Guava method about non-nullness) in the Attribute Filter and Attribute Resolver APS just by removing it (and the blade guard that it implies), but in the end it seemed like better to avoid a nasty surprise if we ever needed to hurry out a release with a more modern Guava than continue to use a safety guard which has never fired.

But there are other much harder (to my mind) cases.  For instance: the SVNResource is full of deprecation warnings (from a third party jar).  It will take some time to pick them apart and right now my instinct is to live with them until V4 and if needs be require a downlevel install if you need that function.  Similarly ISTR that there is something associated with HTTPClient in java support and I'm guessing that Brent isn't going to have much time to look at that in the near future....

It was mostly because of deprecation that I suggested that we add a V4 target.  But even then, slamming a case into V4 isn't of itself enough to allow blind suppression of the warning.

Just M(N)HO.  YMMV and so on.   But I think my bottom line is that being clean in V3.3 although desirable has to be approached carefully.

But thanks for cracking on with JPAR-60.  

Rod



More information about the dev mailing list