which version of checkstyle are you using? there are two errors in parsing the proposed checkstyle file with 5.1;
<!-- <property name="ignoreEnhancedForColon" value="false"/> --> <!-- <module name="OneStatementPerLine"/> --> once i fixed the checkstyle file to work with 5.1, i see that 4935 and 31935 new warnings/errors are introduced in trunk and in my i18n branches, respectively; clearly, this is going to require a large amount of editing to allow use of the proposed rules; many of the new errors I notice (in both trunk and my i18n branches) have to do with whitespace before or after '(', ')', and cast operations; i do not agree with enforcing the presence or absence of whitespace around these constructs; i happen to always use whitespace before and after parens, e.g., the following should produce no checkstyle warning: public int foo ( int a, int b, int c ) { return bar ( a, b, c ); }; i would like whitespace after '{' and before '}' in an array initialization, e.g., both of the following should be permitted: int[] a = new int[] { 1, 2, 3 }; int[] a = new int[] {1, 2, 3}; i would like SimplifyBooleanReturn to be removed; i would like whitespace after BNOT produce a warning, e.g. both ! foo and !foo should be accepted without warning; i would like whitespace after DOT operator to be permissible, e.g., both x.y and x . y should be permitted; i would like empty blocks to be permissible, e.g., the following should be permitted: if ( test ) { /* TBD - handle test is true */ } else { /* TBD - handle test is false */ } i would like the arbitrary line length rule to be removed; i do not agree to 110 line length; or if you insist, i could accept 150; i do not agree with including MultipleVariableDeclarations rule; i routinely define multiple local variables in one statement, e.g., int x, y; i do not agree with requiring LocalFinalVariableName to match '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'; instead, it should continue to match the currently used pattern "^[a-z][a-zA-Z0-9]*$"; why are there two NoWhitespaceAfter rules? <module name="NoWhitespaceAfter"> <property name="tokens" value="ARRAY_INIT"/> </module> <module name="NoWhitespaceAfter"> <property name="allowLineBreaks" value="false"/> <property name="tokens" value="BNOT,DEC,DOT,INC,LNOT,UNARY_MINUS,UNARY_PLUS"/> </module> if you fix the above problems, then i will re-run on trunk and my i18n branch to check if there are any other issues that need to be resolved; On Fri, Feb 3, 2012 at 10:45 AM, Vincent Hennebert <vhenneb...@gmail.com>wrote: > Hi All, > > it is well-known that people are not happy with the Checkstyle file we > have in FOP. And there’s no point enforcing the application of > Checkstyle rules if we don’t agree with them in the first place. > > I’ve finally taken on me to create a new Checkstyle file that follows > modern development practices. I’ve been testing it on my own projects > for a few months now and I’m happy with it, so I’d like to share it with > the community. The idea is that once we’ve reached consensus on the > Checkstyle rules we want to apply, we could set up a no warning policy > and enforce it by running Checkstyle in CI. > > I’m also taking this as an opportunity to propose that we adopt a common > Checkstyle policy to all the sub-projects of XML Graphics. So once we’ve > agreed on a set of rules we would apply them to FOP and XGC immediately, > and eventually also to Batik, and keep them in sync. > > We would also apply the rules to the test files as well as the main > code. Tests are as important as the actual code and there is no reason > why they shouldn’t be checked. > > It is likely that the current code will not be compliant with the new > rules. However, most of them are really just about the syntax, so > I believe it should be fairly straightforward to make the code at least > 90% compliant just by applying Eclipse’s command-line code formatter. > > Please find the Checkstyle file attached. It is based on Checkstyle 5.5 > and basically follows Sun’s recommendations for Java styling with a few > adaptations. What’s noteworthy is the following: > > • Removed checks for Javadoc. What we want is quality Javadoc, and that > is not something that Checkstyle can check. Having Javadoc checks is > counter-productive as it forces us to put {@inheritDoc} everywhere, or > to create truly useless doc like the following: > /** > * Returns the thing. > * @return the thing > */ > public Thing getThing() { > return thing; > } > This is just clutter really. I think it should be left to peer review > to check whether a Javadoc comment is properly written, or whether the > lack thereof is justified. There’s an excellent blog entry from > Torsten Curdt about this: > http://vafer.org/blog/20050323095453/ > • Removed check for file and method lengths. I don’t think it makes > sense to define a maximum size for files and methods. Sometimes > a 10-line method is way too big, sometimes it makes sense to have it > reach 20 lines. Same for files: it’s ok to reach 1000 lines if the > class contains several inner classes. If it doesn’t, then it’s > probably too big. I don’t think there is any definite figure we can > agree on and blindly follow, so I think sizes should be left to peer > review. > • However, I left the check for maximum line length because unreasonably > long lines make the code hard to follow. I increased it to 110 > though to follow the evolution of monitor sizes. But as Peter > suggested me, we probably want to keep it low in order to make > side-by-side comparison easy. > • I added a check for the order of imports; this is to reduce noise in > diffs when committing. I think most of us have configured their IDE to > automatically organise imports when saving changes to a file. This is > a great feature because it allows to keep the list of imports > up-to-date. But in order to avoid constant back and forth changes when > different committers change the same file, I think it makes sense that > we all have the same configuration. I modeled this list after > Jeremias’ one, that I progressively inferred from his commits. > > Please let me know what you think. I’m inclined to follow lazy consensus > on this, and apply the proposed changes if nobody has objected within > 2 weeks. If anybody feels that a formal vote is necessary, feel free to > say so. > > Thanks, > Vincent > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: general-unsubscr...@xmlgraphics.apache.org > For additional commands, e-mail: general-h...@xmlgraphics.apache.org >