Return-Path: 
 <general-return-1764-apmail-xmlgraphics-general-archive=www.apache.org@xmlgraphics.apache.org>
X-Original-To: apmail-xmlgraphics-general-archive@www.apache.org
Delivered-To: apmail-xmlgraphics-general-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 6C29DCB35
	for <apmail-xmlgraphics-general-archive@www.apache.org>;
 Wed, 25 Apr 2012 19:02:50 +0000 (UTC)
Received: (qmail 38163 invoked by uid 500); 25 Apr 2012 19:02:50 -0000
Mailing-List: contact general-help@xmlgraphics.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:general-help@xmlgraphics.apache.org>
List-Unsubscribe: <mailto:general-unsubscribe@xmlgraphics.apache.org>
List-Post: <mailto:general@xmlgraphics.apache.org>
List-Id: <general.xmlgraphics.apache.org>
Reply-To: general@xmlgraphics.apache.org
Delivered-To: mailing list general@xmlgraphics.apache.org
Received: (qmail 38153 invoked by uid 99); 25 Apr 2012 19:02:50 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 25 Apr 2012 19:02:50 +0000
X-ASF-Spam-Status: No, hits=2.2 required=5.0
	tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_NEUTRAL
X-Spam-Check-By: apache.org
Received-SPF: neutral (nike.apache.org: local policy)
Received: from [209.85.213.54] (HELO mail-yw0-f54.google.com) (209.85.213.54)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 25 Apr 2012 19:02:43 +0000
Received: by yhgm50 with SMTP id m50so622962yhg.27
        for <general@xmlgraphics.apache.org>;
 Wed, 25 Apr 2012 12:02:20 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=google.com; s=20120113;
        h=mime-version:in-reply-to:references:from:date:message-id:subject:to
         :content-type:x-gm-message-state;
        bh=JE0lWICtsZL6lTNJ0BMR5JbcJESwCcfbOkWDIGyOjTg=;
        b=U/mUaVXJyecfDCaO9BSReVg3mr+J2CKpVJu/e4OoSzxiyFdm1DEYb6R25wFXmIVYuY
         R8lbyQ1m9HSHLgv/tspx1mUllu6/HgKa13bclABmenDZO7owUtIuESYwkO0ua2GMngCo
         cxDFJUQfV9nbRNAPpRK3O5I4HYFe+crem+3qNFYcvdiTd2G+bCxFZLm4J4QBOuFt/e+f
         7orPKnlVC5BMmFw4V7P/WvB4717JZJEsmHhpkK/P3xA5RIGUw0yj1gYyJJvDmucZK9+1
         KI5fpQ6ne1yR3/DmyT2TGHe01BYEYuoTlkh07hiIpYCtmnnx78OY2Wit6dG//tp8iFUu
         tXrw==
Received: by 10.100.246.16 with SMTP id t16mr1146196anh.3.1335380539792; Wed,
 25 Apr 2012 12:02:19 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.236.145.169 with HTTP; Wed, 25 Apr 2012 12:01:59 -0700 (PDT)
In-Reply-To: <4F98427C.2070603@gmail.com>
References: <4F2C1D25.8010501@gmail.com> <4F981BC1.804@gmail.com>
 <CACQ=j+e=HtOdANPee6hq-GME7RqR8RPoHEZkHb5f2gqjBrCZGA@mail.gmail.com>
 <4F98386B.7000602@gmail.com>
 <CACQ=j+eXdy_w5D+DQF-0hL=6Kg-eih6YGQo3-S2m9EnnjHDMrw@mail.gmail.com>
 <4F98427C.2070603@gmail.com>
From: Glenn Adams <glenn@skynav.com>
Date: Wed, 25 Apr 2012 13:01:59 -0600
Message-ID: 
 <CACQ=j+e2FkL25k5-Qw6Vc=3BB4Bi_+6RhPo_+AaavdPJNoy67A@mail.gmail.com>
Subject: Re: Checkstyle, Reloaded
To: general@xmlgraphics.apache.org
Content-Type: multipart/alternative; boundary=0016e68debaab6dd7d04be858230
X-Gm-Message-State: 
 ALoCoQkJVqjv6mdZrnMG/+n9AnYsef4/a4q4QwfBWp5bjZro7KmHVwZbt0fs0mVZOVqVGF8HVpqI

--0016e68debaab6dd7d04be858230
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Wed, Apr 25, 2012 at 12:29 PM, Vincent Hennebert <vhennebert@gmail.com>w=
rote:

> 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 a=
re
> >> the
> >>> current default in fop?
> >>
> >> The following rules have been removed:
> <snip/>
>
> >> =E2=80=A2 CSOFF and CSOK
> >>
> >
> > i do not accept removing these unless you are willing to remove all rul=
es
> > that trigger a warning/error in the absence of these pragmas
>
> Those are essentially the rules about whitespace. I=E2=80=99ve given reas=
ons
> 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/20120=
2.mbox/%3cCACQ=3Dj+fuosd_5W09LDnNeCbo-rN+2kpsdqnbH752iH1-N+HJdQ@mail.gmail.=
com%3e>
[2] Re: Checkstyle,
Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/20120=
2.mbox/%3cCACQ=3Dj+f6A+iuDhLYbqgGAmim-eqnMgyD3azvwQ0D8a6HH8bQkw@mail.gmail.=
com%3e>
[3] Re: Checkstyle,
Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/20120=
2.mbox/%3cCACQ=3Dj+cEunN8_d0O=3DdUPCHmMsK9+71Pj3f4VYk23xZMrxuMuuA@mail.gmai=
l.com%3e>
[4] Re: Checkstyle,
Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/20120=
2.mbox/%3cCACQ=3Dj+c3ygYneGjUJP+6xXeMW4yS=3D79De=3D48xSZ=3DEqvuR0ofAw@mail.=
gmail.com%3e>
[5] Re: Checkstyle,
Reloaded<http://mail-archives.apache.org/mod_mbox/xmlgraphics-general/20120=
2.mbox/%3cCACQ=3Dj+fqPEePgSxkQ_c01vdAoN7scRcjytcHuw4FvbhzYBmvog@mail.gmail.=
com%3e>


>
>
> >> =E2=80=A2 Double (No idea what it is about. It doesn=E2=80=99t 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=E2=80=99s not Checkstyle=E2=80=99s job to check for th=
at.
>
>
> <snip/>
> >> =E2=80=A2 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/>
> >> =E2=80=A2 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.


> >> =E2=80=A2 WhitespaceAfter: added typecast to follow Sun=E2=80=99s conv=
entions
> >>
> >
> > 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 usin=
g
> > 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=E2=
=80=99t
> 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:
> >>>> =E2=80=A2 EmptyBlock (allow comments)
> >>>> =E2=80=A2 ExplicitInitialization (not automatically fixable)
> >>>> =E2=80=A2 NoWhitespaceAfter with ARRAY_INIT token
> >>>> =E2=80=A2 ParenPad
> >>>>
> >>>> Note that I=E2=80=99m not happy with removing that last rule. I agre=
e with
> >>>> Alexios that a consistent style makes reading and debugging easier.
> That
> >>>> wouldn=E2=80=99t be too bad if the original style were preserved in =
every
> source
> >>>> file, but this will clearly not happen. In fact, the mixing of style=
s
> >>>> 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=E2=80=99s recommendations:
> >>>>
> >>>>
> >>
> http://www.oracle.com/technetwork/java/javase/documentation/codeconventio=
ns-141388.html#475
> >>>> I=E2=80=99d also like to keep the NoWhitespaceAfter rule, as whitesp=
ace 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 1=
10
> >>>> 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
>
>

--0016e68debaab6dd7d04be858230--