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 Fri, 27 Apr 2012 01:09:46 GMT
On Thu, Apr 26, 2012 at 9:19 AM, Vincent Hennebert <vhennebert@gmail.com>wrote:

> On 25/04/12 20:01, Glenn Adams wrote:
> > 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
> >
>
> I saw that. What I would like to know is what you think about the
> readability concerns that have been raised?
>

please provide a link to whichever messages discusses those concerns


>
>
> <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.
>
> Why? We might as well convert them to the prescribed convention.
>

the current convention in FOP is lower case; you are proposing a new
convention; i would prefer to stay with the current convention for log


>
>
> <snip/>
> >>> 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;
>
> Again this is a mere style preference. I’m afraid it doesn’t count
> compared to reasons of readability and convenience for side-by-side
> comparison.
>

sorry, but i disagree; you are arguing your convenience against my
convenience; you are going to insist that is more convenient for you to use
short lines, and i am going to insist that it is more convenient for me to
use long lines;

i proposed a compromise i can live with: 130 line length plus use of CSOFF
in files that do not follow this rule; i will not agree to anything less; i
would suggest you follow the advice given by Chris in [1]:

"I propose that we remove this rule as Glenn suggests and it will avoid lines
being broken in awkward places too."

[1]
http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/201202.mbox/%3cBLU0-SMTP420D413C8764CC374545B5BFB7B0@phx.gbl%3e



>
>
> > 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
>
> The goal is to remove CSOFF altogether. There’s no point having
> Checkstyle rules if anybody can disable them using CSOFF comments.
>

that may be your goal, but it is not my goal; if you want to insist on
applying a style rule that i disagree with in principle, then you will have
to allow for exceptions; a better approach would be to not insist on
applying any rule for which there is not unanimous agreement; however, i am
offering a compromise, which is that you may add a rule which i do no agree
with as long as you do not object to me disabling that rule in files i
author; you can't do both (apply the rule and rule out exclusions)


>
> Files your authored will sooner or later be read and modified by other
> people, so they shouldn’t receive any special treatment.
>

in that case, you should not insist on applying a style rule for which
there is not unanimous agreement


> I could agree to raise the limit to 120, but that’s the absolute
> maximum.
>

i can agree to one of the following regarding line length:

(1) default rule enforces 130, but allows exceptions via CSOFF/CSOK; [i
would point out that one of the most popular IBM 1403 printer, Model 2,
introduced in the early 60s and used with both IBM System/360 and
System/370 had 132 print positions]

or

(2) no rule for line length


>
>
> > 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
>
>
> 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