xmlgraphics-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vincent Hennebert <vhenneb...@gmail.com>
Subject Re: Checkstyle, Reloaded
Date Thu, 26 Apr 2012 15:19:01 GMT
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?


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


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


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

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

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


> 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
View raw message