accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Medinets <david.medin...@gmail.com>
Subject Re: Checkstyle Notes
Date Wed, 24 Dec 2014 03:18:48 GMT
+1. I'm glad the project has matured enough to consider moving in this
direction.

On Tue, Dec 23, 2014 at 1:40 PM, John Vines <vines@apache.org> wrote:
> I share sentiments with Josh. I'm all for a more universally supported
> standard. Going back down to 100 characters feels limiting and I'm not a
> fan, but I'm not going to let that stop me.
>
> On Tue, Dec 23, 2014 at 1:36 PM, Josh Elser <josh.elser@gmail.com> wrote:
>>
>> Thanks for taking the time to do this. I know it can be rather monotonous.
>>
>> I've read over the Google Java style guide before and liked (just about)
>> everything I read. The line limit of 80-100 chars *feels* limiting to me
>> (but I've also been told by others my senior that you very much get used to
>> that limitation and grow to like it).
>>
>> Our existing style recommendation is just that (not a requirement), so
>> tooling in Maven would be important to me for however we might move.
>> Something that will tell me when I did something dumb or knuckle-headed
>> will help to remind me.
>>
>> Christopher wrote:
>>
>>> Devs,
>>>
>>> So, I spent some time yesterday investigating the application of a
>>> CheckStyle configuration to apply to our builds.
>>>
>>> I started with the CheckStyle implementation[1] of the Google Style
>>> Guide[2], and a fully formatted Accumulo code base (according to our
>>> current Eclipse formatter standards[3]). I then configured the
>>> maven-checkstyle-plugin[4][5] to run during a build and built repeatedly,
>>> whittled down some of the strictness of the Google style and fixed some of
>>> our errors, to get at a minimal checkstyle configuration that might be
>>> able
>>> to add some additional checks during our build. I found at least one
>>> bug[6]
>>> doing this and evaluating the resulting warnings, and documented some of
>>> the common problems.
>>>
>>> Now, I'm not necessarily arguing for switching to adopting the Google
>>> Style
>>> Guide for our standards today (we can consider voting on that in a new
>>> thread). However, I think it might be beneficial to adopt the Google Style
>>> Guide, especially because that project has formatters for several standard
>>> IDEs, and because the latest version of CheckStyle has a ruleset which
>>> conforms to it, which enables the Maven tooling. So, if anybody is
>>> interesting in us doing that, I would certainly get behind it (and could
>>> help make it happen: reformatting, merging, and applying build tooling).
>>> One thing I like about the Google Style Guide, in particular, is that it
>>> doesn't just provide tools to enforce it, it also documents the standard
>>> with words and reasoning, so we have something to consult, even if you're
>>> not using an IDE or any of the tools.
>>>
>>> As a result of this, I am going to add something to start enforcing checks
>>> for some of the javadoc problems. We can add more checks later, or we can
>>> adopt the Google Style.
>>>
>>> In the meantime, here's a list of the most common style problems I
>>> observed
>>> (note: these aren't necessarily "bad", just non-conforming):
>>>
>>> Line length violations (we're currently using 160, but Google Style Guide
>>> allows 80 or 100, with 100 enforced in the tools by default; many of our
>>> lines exceed the 160).
>>> Escaped unicode in string literals (especially when they represent a
>>> printable character which should just be inserted directly, instead of
>>> encoded).
>>> Use of lower-case L for long literals (prefer 10L over 10l).
>>> Bad import order, use of wildcards.
>>> Missing empty line between license header and package declaration.
>>> Extra whitespace around generic parameters.
>>> Bad package/class/method/parameter/local variable naming conventions
>>> (like
>>> single character variables outside of loops, or use of
>>> underscore/non-standard camelCase).
>>> Missing optional braces for blocks (see a representatively bad example at
>>> CompactionInfo:lines 74-82).
>>> Keywords out of order from JLS recommendations (e.g. static public vs.
>>> public static).
>>> Overloaded methods aren't grouped together.
>>> Multiple statements per line, multiple variable declarations per line.
>>> Array brackets should be on type (String[] names;), not variable name
>>> (String names[];).
>>> Switch statements sometimes omit default case or fall through.
>>> Distance between local variable declaration and the first time it is used
>>> is sometimes too long (many lines).
>>> All uppercase abbreviations in names aren't avoided (LockID should be
>>> LockId).
>>> Operators should start the next line, when wrapping (like concatenation of
>>> two long string literals).
>>> Empty catch blocks (catch blocks should throw an AssertionError if it's
>>> supposed to be impossible to reach, or a comment about why it's safe to
>>> ignore).
>>> Some files have more than one top-level class. They should be in their own
>>> file.
>>> Braces not on same line as statement (like "} else" with "{" on next
>>> line).
>>>
>>> Missing javadocs on public methods.
>>> Missing paragraph markers(<p>) in javadoc paragraphs.
>>> Missing javadoc sentence descriptions.
>>> Javadoc tags out of order from standards.
>>> Missing html close tags in javadocs (<b>  but no</b>,<ul> 
but no</ul>)
>>> Use of<  and>  in javadoc instead of&lt; and&gt; or {@code ...}
>>>
>>> [1]: https://github.com/checkstyle/checkstyle/blob/master/google_
>>> checks.xml
>>> [2]: https://code.google.com/p/google-styleguide/
>>> [3]:
>>> https://github.com/apache/accumulo/blob/master/contrib/
>>> Eclipse-Accumulo-Codestyle.xml
>>> [4]: http://maven.apache.org/plugins/maven-checkstyle-
>>> plugin/check-mojo.html
>>> [5]: http://stackoverflow.com/a/27359107/196405
>>> [6]: https://issues.apache.org/jira/browse/ACCUMULO-3448
>>>
>>> --
>>> Christopher L Tubbs II
>>> http://gravatar.com/ctubbsii
>>>
>>>

Mime
View raw message