accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Vines <vi...@apache.org>
Subject Re: Checkstyle Notes
Date Tue, 23 Dec 2014 18:40:35 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message