Mailing-List: contact dev-help@cxf.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cxf.apache.org
Received-SPF: pass (athena.apache.org: domain of jasonmpell@gmail.com
 designates 209.85.215.47 as permitted sender)
MIME-Version: 1.0
Sender: jasonmpell@gmail.com
In-Reply-To: <519B4032.7020108@gmail.com>
References: <519B2A46.40105@die-schneider.net>
	<303455001.451369128519819.JavaMail.root@shefa>
	<519B4032.7020108@gmail.com>
Date: Tue, 21 May 2013 20:46:16 +1000
Message-ID: 
 <CACBZuv7g4CK64vGEFKu85MF+jZxFqRpzvLoazfMREn6Ya7J9XA@mail.gmail.com>
Subject: Re: Code style
From: Jason Pell <jason@pellcorp.com>
To: dev@cxf.apache.org
Content-Type: multipart/alternative; boundary=14dae93d9348a9e65b04dd38283c

--14dae93d9348a9e65b04dd38283c
Content-Type: text/plain; charset=UTF-8

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.


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

--14dae93d9348a9e65b04dd38283c--