xmlgraphics-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Glenn Adams <gl...@skynav.com>
Subject Re: Checkstyle, Reloaded
Date Wed, 25 Apr 2012 19:01:59 GMT
On Wed, Apr 25, 2012 at 12:29 PM, Vincent Hennebert <vhennebert@gmail.com>wrote:

> On 25/04/12 19:03, Glenn Adams wrote:
> > On Wed, Apr 25, 2012 at 11:46 AM, Vincent Hennebert <
> vhennebert@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?
>

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+HJdQ@mail.gmail.com%3e>
[2] Re: Checkstyle,
Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cCACQ=j+f6A+iuDhLYbqgGAmim-eqnMgyD3azvwQ0D8a6HH8bQkw@mail.gmail.com%3e>
[3] Re: Checkstyle,
Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cCACQ=j+cEunN8_d0O=dUPCHmMsK9+71Pj3f4VYk23xZMrxuMuuA@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=EqvuR0ofAw@mail.gmail.com%3e>
[5] Re: Checkstyle,
Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cCACQ=j+fqPEePgSxkQ_c01vdAoN7scRcjytcHuw4FvbhzYBmvog@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 <
> vhennebert@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-unsubscribe@xmlgraphics.apache.org
> For additional commands, e-mail: general-help@xmlgraphics.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message