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
<[email protected]>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>