Please can we wait to reach an agreement before making any changes? I realize the thread in @general hasn’t made any progress in 3 weeks and I’ll try to revive it, but making changes now will just make things more confusing.
In particular, I think we want to all use the same version of Checkstyle in order to ensure consistency. ATM Checkstyle 5.1 is used but when the new set of rules is in place I think we will all want to switch to Checkstyle 5.5. The checkstyle.location however is a good move, I like it. Thanks, Vincent On 02/03/12 00:30, Glenn Adams wrote: > In order to proceed on Vincent's proposed new checkstyle rules [1], I've > committed support for checkstyle-5.5 and the new rules [2]. > > [1] > http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3c4f2c1d25.8010...@gmail.com%3e > [2] http://svn.apache.org/viewvc?view=revision&revision=1296000 > > More specifically, I parameterized build.xml to use either checkstyle-5.1 > or checkstyle-5.5, where the (current) default is checkstyle-5.1. Which > version is used can be controlled by setting the following ant parameters: > > - checkstyle.location (default: ${lib-tools}/checkstyle-all-5.1.jar) > - checkstyle.config (default: ${basedir}/checkstyle-5.1.xml) > > I'm specifying the following in my build-local.properties in order to > select checkstyle-5.5: > > checkstyle.config = ${basedir}/checkstyle-5.5.xml > checkstyle.location = ${basedir}/lib/build/checkstyle-5.5-all.jar > > I manually merged the new rules proposed by Vincent with the old set of > checkstyle-5.1 rules to produce a new checkstyle-5.5.xml config file as > follows: > > - if adding a new rule did not produce new errors, i added; otherwise, i > added a commented out rule with a comment indicating the number of new > errors > - if changing an existing rule did not produce new errors, i changed it; > otherwise, i added a comment indicating the number of new errors produced > - for existing rules not included in [1], I removed some but retained a > few, about which see the additional notes below; > - i reordered the rules to be alphabetical, and added some divider > comments to make it a little easier to read the separate rules > > As of this commit, both checkstyle-5.1 and -5.5 do not produce any errors; > however, in order to make full use of all the proposed new rules or changed > rules some code will need to change or the proposed new rule or rule change > dropped. An ordered list of the number of new errors that would be produced > (by rule) is as follows: > > ParenPad 16666 > MethodParamPad 4316 > WhitespaceAfter 2203 > ExplicitInitialization 795 > ImportOrder 255 > NoWhitespaceAfter 183 > NewLineAtEndOfFile 111 > UnusedImports 80 > MultipleVariableDeclarations 78 > RegexpSingleLine 46 > OneStatementPerLine 43 > RedundantImport 22 > DefaultComesLast 9 > RightCurly 5 > RedundantModifier 3 > GenericWhitespace 1 > NoWhitespaceBefore 1 > > For those rules above with less than, say 300 errors, the errors can be > addressed (in subsequent commits) and the rule enabled. For those rules > above producing more than 500 new errors, I would suggest we not attempt to > implement the rules, particularly since we don't have full consensus on > their use. > > Here are my notes on the rule merge process: > > ConstantName > 1. current format ^([A-Z](_?[A-Z0-9]+)*)|(log)$ > 2. proposed (default) format ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$ > 3. problems with current code base > (a) requires at least one '_' > (b) requires length > 1 > (b) doesn't admit 'log' > 4. conclusion: retain existing format > > DefaultComesLast > 1. produces 9 new errors > > DoubleCheckedLockinng > 1. left in, even though not present in proposed rules > > EmptyBlock > 1. left LITERAL_CATCH, even though removed in proposed rule > 2. changing block policy to default (stmt) instead of text produces 110 > new errors > > EmptyForIteratorPad > 1. removed (not present in proposed rules) > > EqualsHashCode > 1. left in, even though not present in proposed rules > > ExplicitInitialization > 1. produces 795 new errors > > FileLength > 1. removed (not present in proposed rules) > > GenericWhitespace > 1. produces 1 new error > > ImportOrder > 1. produces 255 new errors > > InnerAssignment > 1. left in, even though not present in proposed rules (I propose we > remove this rule, but want to confirm with the group before doing so) > > Javadoc{Method,Type,Variable} > 1. removed (not present in proposed rules) > > LineLength > 1. removed, as suggested by cbowditch [3] > > [3] > http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cblu0-smtp420d413c8764cc374545b5bfb...@phx.gbl%3e > > MethodLength > 1. removed (not present in proposed rules) > > MethodParamPad > 1. produces 4316 new errors > > MultipleVariableDeclarations > 1. produces 78 new errors > > NewLineAtEndOfFile > 1. produces 111 new errors > > NoWhitespaceAfter > 1. changing allowLineBreaks to false produces 183 new errors > > NoWhitespaceBefore > 1. changing allowLineBreaks to false produces 1 new error > > OneStatementPerLine > 1. produces 43 new errors > > ParameterNumber > 1. removed (not present in proposed rules) > > ParenPad > 1. produces 16666 new errors > > RedundantImport > 1. produces 22 new errors > > RedundantModifier > 1. adding INTERFACE_DEF (via default tokens) produces 3 new errors > > RegexpSingleLine > 1. produces 46 new errors > > RightCurly > 1. adding LITERAL_IF (via default tokens) produces 5 new errors > > UnusedImports > 1. produces 80 new errors > > VisibilityModifier > 1. left in, even though not present in proposed rules > > WhitespaceAfter > 1. adding TYPECAST (via default tokens) produces 2203 new errors > > WhitespaceAround > 1. left BAND, DIV, and STAR tokens, even though removed in proposed rule >
