accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Elser <josh.el...@gmail.com>
Subject Re: Checkstyle Notes
Date Tue, 23 Dec 2014 18:36:25 GMT
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