You are viewing a plain text version of this content. The canonical link for it is here.
Posted to general@xmlgraphics.apache.org by Vincent Hennebert <vh...@gmail.com> on 2013/02/27 22:38:49 UTC

Checkstyle, Epilogue

Hi All,

An extremely long time ago I proposed on this list [1] a new set of
Checkstyle rules that we would all agree to follow, so that we can
enforce a no warning policy. Glenn expressed disagreements with some of
the rules.

[1] http://markmail.org/thread/wnevlowocmlcm6xa

A very long time ago I had a Skype discussion with Glenn in order to try
and find a common ground; We agreed that, pending some action from my
part, we would settle on a compromise.

A long time ago I undertook that action and submitted my results to
Glenn, who was ok to proceed with the compromise we had found.

So, today, I’d like to propose this compromise to the rest of the
community and ask if everybody is ok with it. Mainly, we keep the rules
about whitespace that I suggested last year, but we introduce an
exception regarding line length: files originally authored by Glenn are
allowed to disable that rule.

The differences with the Checkstyle file currently in FOP are the
following (see [2] for rule descriptions):
• remove SuppressWithNearbyCommentFilter
• remove EqualsHashCode
• remove NoWhiteSpaceAfter for ARRAY_INIT
• add LineLength
• add MethodParamPad
• add ParenPad
• WhiteSpaceAfter: add TYPECAST
• WhiteSpaceAround: remove DIV and STAR

[2] http://checkstyle.sourceforge.net/availablechecks.html

I would also like to add ExplicitInitialization eventually, once I’ve
figured out an automatic way of removing the existing ones.

Some source files (e.g. o.a.f.complexscripts.bidi.BidiClass) were
automatically generated and are not meant to be human-readable,
therefore I will exclude them from the checks.

I’ll apply lazy consensus and, if nobody objects within 3 working days,
I will start to progressively apply the new rules and enable them in
Checkstyle. Then we can modify Gump to automatically run Checkstyle for
us after every commit.


Thanks,
Vincent

RE: Checkstyle, Epilogue

Posted by Simon Steiner <si...@gmail.com>.
Hi,

Ignore my comment about DoubleCheckedLockingCheck

Thanks

-----Original Message-----
From: Simon Steiner [mailto:simonsteiner1984@gmail.com] 
Sent: Thursday, February 28, 2013 10:37 AM
To: general@xmlgraphics.apache.org
Subject: RE: Checkstyle, Epilogue

HI,

With checkstyle 5.6 and your new config i get
build.xml:1052: Unable to create a Checker: cannot initialize module TreeWalker - Unable to instantiate DoubleCheckedLockingCheck

This changed fixed that
http://svn.apache.org/viewvc?view=revision&revision=1428897

Config has been reordered so diff is impossible

Thanks

-----Original Message-----
From: Vincent Hennebert [mailto:vhennebert@gmail.com] 
Sent: Wednesday, February 27, 2013 9:39 PM
To: general@xmlgraphics.apache.org
Subject: Checkstyle, Epilogue

Hi All,

An extremely long time ago I proposed on this list [1] a new set of Checkstyle rules that we would all agree to follow, so that we can enforce a no warning policy. Glenn expressed disagreements with some of the rules.

[1] http://markmail.org/thread/wnevlowocmlcm6xa

A very long time ago I had a Skype discussion with Glenn in order to try and find a common ground; We agreed that, pending some action from my part, we would settle on a compromise.

A long time ago I undertook that action and submitted my results to Glenn, who was ok to proceed with the compromise we had found.

So, today, I’d like to propose this compromise to the rest of the community and ask if everybody is ok with it. Mainly, we keep the rules about whitespace that I suggested last year, but we introduce an exception regarding line length: files originally authored by Glenn are allowed to disable that rule.

The differences with the Checkstyle file currently in FOP are the following (see [2] for rule descriptions):
• remove SuppressWithNearbyCommentFilter • remove EqualsHashCode • remove NoWhiteSpaceAfter for ARRAY_INIT • add LineLength • add MethodParamPad • add ParenPad • WhiteSpaceAfter: add TYPECAST • WhiteSpaceAround: remove DIV and STAR

[2] http://checkstyle.sourceforge.net/availablechecks.html

I would also like to add ExplicitInitialization eventually, once I’ve figured out an automatic way of removing the existing ones.

Some source files (e.g. o.a.f.complexscripts.bidi.BidiClass) were automatically generated and are not meant to be human-readable, therefore I will exclude them from the checks.

I’ll apply lazy consensus and, if nobody objects within 3 working days, I will start to progressively apply the new rules and enable them in Checkstyle. Then we can modify Gump to automatically run Checkstyle for us after every commit.


Thanks,
Vincent



---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org


RE: Checkstyle, Epilogue

Posted by Simon Steiner <si...@gmail.com>.
HI,

With checkstyle 5.6 and your new config i get
build.xml:1052: Unable to create a Checker: cannot initialize module TreeWalker - Unable to instantiate DoubleCheckedLockingCheck

This changed fixed that
http://svn.apache.org/viewvc?view=revision&revision=1428897

Config has been reordered so diff is impossible

Thanks

-----Original Message-----
From: Vincent Hennebert [mailto:vhennebert@gmail.com] 
Sent: Wednesday, February 27, 2013 9:39 PM
To: general@xmlgraphics.apache.org
Subject: Checkstyle, Epilogue

Hi All,

An extremely long time ago I proposed on this list [1] a new set of Checkstyle rules that we would all agree to follow, so that we can enforce a no warning policy. Glenn expressed disagreements with some of the rules.

[1] http://markmail.org/thread/wnevlowocmlcm6xa

A very long time ago I had a Skype discussion with Glenn in order to try and find a common ground; We agreed that, pending some action from my part, we would settle on a compromise.

A long time ago I undertook that action and submitted my results to Glenn, who was ok to proceed with the compromise we had found.

So, today, I’d like to propose this compromise to the rest of the community and ask if everybody is ok with it. Mainly, we keep the rules about whitespace that I suggested last year, but we introduce an exception regarding line length: files originally authored by Glenn are allowed to disable that rule.

The differences with the Checkstyle file currently in FOP are the following (see [2] for rule descriptions):
• remove SuppressWithNearbyCommentFilter • remove EqualsHashCode • remove NoWhiteSpaceAfter for ARRAY_INIT • add LineLength • add MethodParamPad • add ParenPad • WhiteSpaceAfter: add TYPECAST • WhiteSpaceAround: remove DIV and STAR

[2] http://checkstyle.sourceforge.net/availablechecks.html

I would also like to add ExplicitInitialization eventually, once I’ve figured out an automatic way of removing the existing ones.

Some source files (e.g. o.a.f.complexscripts.bidi.BidiClass) were automatically generated and are not meant to be human-readable, therefore I will exclude them from the checks.

I’ll apply lazy consensus and, if nobody objects within 3 working days, I will start to progressively apply the new rules and enable them in Checkstyle. Then we can modify Gump to automatically run Checkstyle for us after every commit.


Thanks,
Vincent


---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org


Re: Checkstyle, Epilogue

Posted by Glenn Adams <gl...@skynav.com>.
On Wed, Feb 27, 2013 at 2:38 PM, Vincent Hennebert <vh...@gmail.com>wrote:

> So, today, I’d like to propose this compromise to the rest of the
> community and ask if everybody is ok with it. Mainly, we keep the rules
> about whitespace that I suggested last year, but we introduce an
> exception regarding line length: files originally authored by Glenn are
> allowed to disable that rule.
>

What I said was:

> In any case, I can accept the 120 limit on files authored by others while
I
> will continue to disable the limit for files created by myself.
>
> Also, I think you will need to update the style guide docs to reflect
these
> new enforced styles. For the line length, you can write something like:
>
> "A default line length limit of 120 is observed, and files that were
> created using this limit should continue to observe this limit. Authors of
> new files are encouraged to use this limit as well, although it may be
> overridden without prejudice."

So I did not propose that the exception apply only to myself.

Re: Checkstyle, Epilogue

Posted by Vincent Hennebert <vh...@gmail.com>.
Ok, so it seems that consensus has been reached. I’ll update the
Checkstyle config file progressively as I implement the new rules. I’ll
also update the documentation on the website as suggested by Glenn.

Thanks,
Vincent

On 02/27/2013 10:38 PM, Vincent Hennebert wrote:
> Hi All,
>
> An extremely long time ago I proposed on this list [1] a new set of
> Checkstyle rules that we would all agree to follow, so that we can
> enforce a no warning policy. Glenn expressed disagreements with some of
> the rules.
>
> [1] http://markmail.org/thread/wnevlowocmlcm6xa
>
> A very long time ago I had a Skype discussion with Glenn in order to try
> and find a common ground; We agreed that, pending some action from my
> part, we would settle on a compromise.
>
> A long time ago I undertook that action and submitted my results to
> Glenn, who was ok to proceed with the compromise we had found.
>
> So, today, I’d like to propose this compromise to the rest of the
> community and ask if everybody is ok with it. Mainly, we keep the rules
> about whitespace that I suggested last year, but we introduce an
> exception regarding line length: files originally authored by Glenn are
> allowed to disable that rule.
>
> The differences with the Checkstyle file currently in FOP are the
> following (see [2] for rule descriptions):
> • remove SuppressWithNearbyCommentFilter
> • remove EqualsHashCode
> • remove NoWhiteSpaceAfter for ARRAY_INIT
> • add LineLength
> • add MethodParamPad
> • add ParenPad
> • WhiteSpaceAfter: add TYPECAST
> • WhiteSpaceAround: remove DIV and STAR
>
> [2] http://checkstyle.sourceforge.net/availablechecks.html
>
> I would also like to add ExplicitInitialization eventually, once I’ve
> figured out an automatic way of removing the existing ones.
>
> Some source files (e.g. o.a.f.complexscripts.bidi.BidiClass) were
> automatically generated and are not meant to be human-readable,
> therefore I will exclude them from the checks.
>
> I’ll apply lazy consensus and, if nobody objects within 3 working days,
> I will start to progressively apply the new rules and enable them in
> Checkstyle. Then we can modify Gump to automatically run Checkstyle for
> us after every commit.
>
>
> Thanks,
> Vincent
>

---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org