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 15:03:39 GMT
On 21/05/13 11:51, Sergey Beryozkin wrote:
> 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
Hmm... I've caught myself doing few up to 110 lines today :-)

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


Mime
View raw message