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 04:58:14 GMT
https://issues.apache.org/jira/browse/ACCUMULO-1158 is the ticket
covering the previous work. I remember somehow I had just turned one
check on and wrote a script file to remove line endings. I understood
totally why the change was rejected. Unfortunately I don't have any
copies of that version.

On Tue, Dec 23, 2014 at 11:21 PM, Christopher <ctubbsii@apache.org> wrote:
> David, I know you made a previous pass at this. I looked for your
> checkstyle rules from back then, but it was in svn, so I gave up. You
> wouldn't happen to have a copy of those rulesets you previously applied, in
> a form I could easily cross-reference, would you?
>
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
> On Tue, Dec 23, 2014 at 10:18 PM, David Medinets <david.medinets@gmail.com>
> wrote:
>
>> +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