Adjusted the thread subject. Was: Vote: Add Checkstyle roleset and make code cleanups!
2010/7/8 Peter Roßbach <p...@objektpark.de>: > Hi, > > after the discussion about code style > (http://tomcat.markmail.org/thread/2c7lkzmpcuxqpgjj), I think that we must > vote for this fix: > > https://issues.apache.org/bugzilla/show_bug.cgi?id=49268 > It includes a starting point for a very limited checkstyle ruleset. > > +1: progressively introduce checkstyle with checks for which a consensus > exists (first TabSpacePolicy, @Version format) > 0 : More discussion needed > -1: no checkstyle integration. Tomcat shoudn't have code style rules 1. What is TabSpacePolicy? It is not listed at: http://checkstyle.sourceforge.net/availablechecks.html There is FileTabCharacter though. http://checkstyle.sourceforge.net/config_whitespace.html#FileTabCharacter 2. There is NO consensus on @Version format. Last time when it was discussed, about a year ago, there was slight agreement to get rid of those tags at all. http://marc.info/?t=124692531300003&r=1&w=2 Though, when I actually was performing the change recently I went for a more safe route of just replacing $Date$ with $Id$. One caveat with $Id$ that I noted in the last several months: when filename is long, "@version $Id$" can occupy more than 80 characters and is wrapped when reformatting the file in IDE, thus breaking this keyword. In several such cases I replaced $Id$ with $Revision$. 3. From message in "Re: r960104" thread: http://markmail.org/message/rkznrp2cnfkd4eob: > FileTabCharacter -> currently 146 failures In what packages are those files? Fixing them can be discussed and done first, before enabling any checkstyle nags in the project. 4. Is it possible to exclude some packages from checkstyle checks? E.g., org.apache.tomcat.util.bcel ? 5. Is there experience whether checkstyle checks run fast, or there are noticeable delays? The "Re: r960104" thread was about preventing commits that have wrong whitespace. It probably means that checkstyle is run automatically by IDE, or by the buildbot. Do others have positive experience with such configurations? Based on the above [x] 0 : More discussion needed I am +1 if someone else wants to add a separate "checkstyle" target to build.xml. I do not mind against checks that already succeed for the existing code. Though if they always succeed they are not really useful. Regarding the checks that fail -- before enabling the check I would like to discuss whether we can fix the code in TC7, and whether we can backport the fix. At least the affected files have to be listed. Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
