cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sergey Beryozkin <sberyoz...@gmail.com>
Subject Re: Code style
Date Tue, 21 May 2013 10:51:57 GMT
On 21/05/13 11:46, Jason Pell wrote:
> since checkstyle enforces 120 already, it would seem sensible to keep with
> that, otherwise if you go for 110 or 100 there are likely to be a heap of
> build breaks.
That would be of the least concern to me - there will be a massive 
number of build breaks in the DOSGi code as a consequence of getting the 
rules applied to it too but they will get fixed.

But as I said I don't really care about the 110 vs 120 limit; I prefer 
the shorter lines either way

Sergey


>
>
> On Tue, May 21, 2013 at 7:36 PM, Sergey Beryozkin <sberyozkin@gmail.com>wrote:
>
>> Hi Amichai
>>
>> On 21/05/13 10:28, A. Rothman wrote:
>>
>>>
>>> I'm glad you brought styling up for discussion, since there are various
>>> inconsistencies in the current code base:
>>>
>>> - Line lengths are indeed one issue I've noticed - I'd go for 120 chars
>>> per line as well, though it's not as important as being consistent.
>>> There are currently lines that are broken also at less than 100 chars,
>>> at awkward places, for no apparent reason.
>>>
>>> - Whitespace: there are currently places where tabs are used for
>>> indentation instead of spaces, as well as many trailing spaces (which
>>> were introducing many artificial diffs in patches I submitted).
>>>
>>> - There are occasional blank lines added at unhelpful locations, e.g.
>>> before the closing brace of a method, or between two closing braces of
>>> two nested blocks.
>>>
>>> - Some method definitions have parameter lists stacked with one
>>> parameter per line with a common left-aligned margin, whereas others use
>>> regular line-continuation rules and indentation (I personally much
>>> prefer the latter).
>>>
>>> - Inconsistent naming of variables, even of the same type and meaning in
>>> very adjacent code.
>>>
>>> - while javadocs are mostly missing, also the existing ones often don't
>>> conform to the standard javadoc conventions.
>>>
>>> - Various other inconsistencies I can't remember at the moment, but
>>> which made review and work on the code harder for a newcomer (and would
>>> make maintenance harder and more error-prone for existing devs as well).
>>>
>>> It would be great to do a one-time sweep and fix all the existing
>>> inconsistencies and respective rules, once the standard is decided upon,
>>> and to better enforce them in the future.
>>>
>>
>> This is the result of the DOSGi codebase having no style enforced at all,
>> I'd suggest to copy the CXF rules there
>>
>> Hi Christian: IMHO the longer the lines the more difficult they are to
>> read, though I guess much depends on the screen resolution and how many
>> editors are open :-), etc... I probably don't mind if it's 110 or 120
>> limit, I'd try to keep it within 100 anyway :-)
>>
>> Sergey
>>
>>
>>> Disclaimer: I'm new here, and haven't reviewed the entire CXF code base,
>>> but focused mainly on the DOSGi subproject, so I don't know how much of
>>> this applies elsewhere.
>>>
>>> Christian - could you please also add a link to the checkstyle/pmd rules
>>> in the guidelines page?
>>>
>>> Amichai
>>>
>>> On 05/21/2013 11:03 AM, Christian Schneider wrote:
>>>
>>>> Hi all,
>>>>
>>>> at the moment our rules for code styles are a bit hidden. When Amichai
>>>> asked me about the rules at the CXF code base it took me some time to
>>>> find the
>>>> formatter for eclipse.  I added a link to the Coding Guidelines in the
>>>> wiki. https://cwiki.apache.org/**confluence/display/CXF/Coding+**
>>>> Guidelines<https://cwiki.apache.org/confluence/display/CXF/Coding+Guidelines>
>>>>
>>>> When I checked it I found that the code formatter sets the line length
>>>> to 110 characters while the checkstyle rule checks for 120 characters.
>>>> I think we should set the same count for both rules.
>>>>
>>>> Which do you prefer?
>>>>
>>>> I favour 120 characters.
>>>>
>>>> Christian
>>>>
>>>> Btw. the checkstyle rules are here:
>>>> http://svn.apache.org/repos/**asf/cxf/build-utils/trunk/**
>>>> buildtools/src/main/resources/**cxf-checkstyle.xml<http://svn.apache.org/repos/asf/cxf/build-utils/trunk/buildtools/src/main/resources/cxf-checkstyle.xml>
>>>>
>>>>
>>>>
>>>
>>>
>>
>


-- 
Sergey Beryozkin

Talend Community Coders
http://coders.talend.com/

Blog: http://sberyozkin.blogspot.com

Mime
View raw message