On Wed, Apr 25, 2012 at 12:29 PM, Vincent Hennebert <[email protected]>wrote:
> On 25/04/12 19:03, Glenn Adams wrote: > > On Wed, Apr 25, 2012 at 11:46 AM, Vincent Hennebert < > [email protected]>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? > i did; see my responses at [1-5]: [1] Re: Checkstyle, Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cCACQ=j+fuosd_5w09ldnnecbo-rn+2kpsdqnbh752ih1-n+h...@mail.gmail.com%3e> [2] Re: Checkstyle, Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cCACQ=j+f6a+iudhlybqggamim-eqnmgyd3azvwq0d8a6hh8b...@mail.gmail.com%3e> [3] Re: Checkstyle, Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cCACQ=j+cEunN8_d0O=dupchmmsk9+71pj3f4vyk23xzmrxum...@mail.gmail.com%3e> [4] Re: Checkstyle, Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cCACQ=j+c3ygYneGjUJP+6xXeMW4yS=79De=48xSZ=eqvur0o...@mail.gmail.com%3e> [5] Re: Checkstyle, Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cCACQ=j+fqpeepgsxkq_c01vdaon7scrcjytchuw4fvbhzybm...@mail.gmail.com%3e> > > > >> • 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. > I would prefer to leave it as is currently used. > >> • 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. ah, then i guess i noticed many existing uses that did not put whitespace after the typecast; if you wish to enforce this and also will make the changes to existing code, then i can agree > > > 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? > as i have stated numerous times, i use an editor (emacs) that makes long lines easy to handle, so i don't have a problem with them; on my (15" laptop) screen, i get 200 columns before a wrap; i prefer to *not* break a statement artificially into lines simply due to an arbitrary line length limit; if you don't mind me using my style in files i author (with CSOFF to disable), then i can accept a shorter limit, e.g., i believe someone proposed 130 but personally, i think it best not to enforce any limit > > Thanks, > Vincent > > > >>> On Wed, Apr 25, 2012 at 9:44 AM, Vincent Hennebert < > [email protected] > >>> 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: [email protected] > For additional commands, e-mail: [email protected] > >
