On 25/04/12 19:03, Glenn Adams wrote: > On Wed, Apr 25, 2012 at 11:46 AM, Vincent Hennebert > <vhenneb...@gmail.com>wrote: > >> On 25/04/12 17:03, Glenn Adams wrote: >>> how does this differ from the current checkstyle-5.5.xml rules that are >> the >>> current default in fop? >> >> The following rules have been removed: <snip/>
>> • CSOFF and CSOK >> > > i do not accept removing these unless you are willing to remove all rules > that trigger a warning/error in the absence of these pragmas Those are essentially the rules about whitespace. I’ve given reasons what I think we should keep some of them. Could you comment on them? >> • Double (No idea what it is about. It doesn’t appear in the list of >> available checks for Checkstyle 5.5.) >> > > the full name is DoubleCheckedLocking, which is documented at > > http://checkstyle.sourceforge.net/config_coding.html#DoubleCheckedLocking Ha, ok. I think it’s not Checkstyle’s job to check for that. <snip/> >> • EqualsHashCode >> > > i think this should stay, since it is part of the object contract, and > exceptions (via CSOFF/CSOK) need to be explicitly documented Same here. I think Checkstyle should be restricted to, well, checking style. <snip/> >> • ConstantName: removed log exception >> > > could you elaborate? Static final log fields will have to be made uppercase. >> • WhitespaceAfter: added typecast to follow Sun’s conventions >> > > i don't accept this, particularly since it is widely used in FOP code (and > I always use whitespace after typecast) ?? Using a whitespace after a cast is precisely what this rule enforces. > i also don't accept changing LineLength back to 110; i believe someone > proposed 130, which I can accept as long as i can disable entirely using > CSOFF; i would prefer *no* limit I (and others) have given good reasons why the line length should be limited. Surely those reasons prevail over mere style preference, don’t they? Thanks, Vincent >>> On Wed, Apr 25, 2012 at 9:44 AM, Vincent Hennebert <vhenneb...@gmail.com >>> wrote: >>> >>>> Ok, reviving a thread that has been dormant for too long. >>>> >>>> Attached is an updated version of the proposed Checkstyle configuration. >>>> I removed/relaxed the following rules: >>>> • EmptyBlock (allow comments) >>>> • ExplicitInitialization (not automatically fixable) >>>> • NoWhitespaceAfter with ARRAY_INIT token >>>> • ParenPad >>>> >>>> Note that I’m not happy with removing that last rule. I agree with >>>> Alexios that a consistent style makes reading and debugging easier. That >>>> wouldn’t be too bad if the original style were preserved in every source >>>> file, but this will clearly not happen. In fact, the mixing of styles >>>> has already started after the complex scripts patch was applied. I still >>>> removed the rule though. >>>> >>>> However, I left the MethodParamPad rule in order to remain compliant >>>> with Sun’s recommendations: >>>> >>>> >> http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-141388.html#475 >>>> I’d also like to keep the NoWhitespaceAfter rule, as whitespace after >>>> unary operators increases too much the risk of misreading the statement >>>> IMO. >>>> >>>> Finally, I left the LineLength rule to 110. Long lines impede code >>>> readability too much IMO. They also make side-by-side comparison harder. >>>> I note that some even recommend to leave the check to 100. I think 110 >>>> should be an acceptable compromise. >>>> >>>> Please let me know what you think. >>>> Thanks, >>>> Vincent --------------------------------------------------------------------- To unsubscribe, e-mail: general-unsubscr...@xmlgraphics.apache.org For additional commands, e-mail: general-h...@xmlgraphics.apache.org
