I think we should just have the automated patch process check for
style rules that we've already fixed. This should prevent regressions.
Then the nightly build can run checkstyle to report all the style
errors still existing in trunk.
FWIW, running just this rule:
<module name="Indentation">
<property name="basicOffset" value="2" />
<property name="caseIndent" value="2" />
</module>
with trunk still yields 1799 indentation errors.
Nige
On Apr 17, 2007, at 3:12 PM, Doug Cutting wrote:
Nigel Daley wrote:
Hmm, we're still at almost 9000 style errors. I think it will be
too difficult for people to figure out where their problems are.
Perhaps I start by having the patch process generate the
checkstyle html file which can be looked at by committers. Thoughts?
Sure, that'd be good. But patches still shouldn't increase the
number.
Looking at the current output, the most common things seem to be:
% grep '^<td>' < build/test/checkstyle-errors.html | grep -v href |
sed 's|<td>[0-9]*</td>$||' | sed 's|<[/]*td>||g' | sort | uniq -c |
sort -nr | head -20
1321 '(' is followed by whitespace.
1299 ')' is preceded with whitespace.
1120 Line is longer than 80 characters.
477 First sentence should end with a period.
475 'if' construct must use '{}'s.
215 Redundant 'public' modifier.
196 ',' is not followed by whitespace.
168 '{' should be on the previous line.
139 Array brackets at illegal position.
123 method def modifier at indentation level 4 not at correct
indentation, 2
121 ';' is preceded with whitespace.
119 Line contains a tab character.
118 Must have at least one statement.
117 '!' is followed by whitespace.
90 method def child at indentation level 6 not at correct
indentation, 4
77 '}' should be on the same line.
52 method def rcurly at indentation level 4 not at correct
indentation, 2
52 if child at indentation level 8 not at correct indentation, 6
51 Inner assignments should be avoided.
49 ';' is followed by whitespace.
The first two of these concern whitespace, and could be fixed
automatically w/o breaking patches.
Doug