My proposals for changes to the default sun style rules:

checkstyle.maxlinelen=100
I agree with Mike and Simon that the line length should be longer. 100 is fairly standard in other projects. (comment blocks should still be 80 however it is not enforceable with checkstyle). Martin brought up a good point about side by side diff needing a narrow format, but I think that artifically braking lines needlessly does more dammage than having to scroll a little bit in a diff. Besides, if a diff user is unhappy with line length, they can use a higher resolution, but if somone doe not like many broken lines, they have no recourse.

checkstyle.pattern.publicmember=^[a-z][a-zA-Z0-9]*$
The default is ^f[A-Z][a-zA-Z0-9]*$ which is apparently for EJB but is just plain stupid in the HttpClient context.

checkstyle.pattern.package=^[a-z]+(\.[a-z]*)*$
The default is ^[a-z]+(\.[a-zA-Z_][a-zA-Z_0-9]*)*$ which allows caps, underscores and numbers. HttpClient does not use these symbols, and we shouldn't start using them.

checkstyle.header.file=license.regexp
checkstyle.header.regexp=true
http://cvs.apache.org/viewcvs/jakarta-commons/httpclient/license.regexp?rev=HEAD&content-type=text/vnd.viewcvs-markup
This is to enforce that all source files have the commons license, the usual CVS keywords, and a current date that makes sense. (this is already in CVS)

checkstyle.ignore.maxlinelen=Header:
This is to prevent any warning for the expansion of the $Header$ cvs keyword which is always very long. I wish I had a better regexp for this that 1) works and 2) does not itself get expanded as a keyword when the checkstyle.properties file is commited. (this is already in CVS)

checkstyle.tab.width=4
This does not really matter for us as tabs are turned off, but the world would be a better place if the standard size for a tab was 4 spaces.

Jandalf.


Mike Bowler wrote:

I am going through the code and changing it to conform to the style as per checkstyle. There are two places that I think we may want to be a little more lenient than the defaults and so I'm putting them here for discussion.

1) Line length of 80

The number 80 originally came from the days when printers could only print 80 columns. These days, that number doesn't make nearly as much sense. While I think we still want to have a maximum length, I believe that 80 is too short.

In most of the places that we are exceeding the limit, we are just over by a bit (between 80 and 90). I propose that we change the max length to 100. This is still short enough that nobody should have to scroll to see the source but long enough that we aren't artificially breaking lines.

2) Instance variable names

Some of the code uses leading underscores for variable names but this isn't allowed by checkstyle. The pattern it checks for is ^[a-z][a-zA-Z0-9]*$

I find significant benefit to being able to quickly distinguish instance variables from local variables and the leading underscore lets me do that easily. (I personally prefer trailing underscores to leading ones but I can live with either)

I propose that we change the pattern for instance variables to ^_?[a-z][a-zA-Z0-9]*$ so that we will allow leading underscores but will not insist on it.

Comments?


--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to