I agree with all that! :-)

Gary


On Feb 9, 2017 7:05 PM, "Matt Sicker" <boa...@gmail.com> wrote:

I was browsing through the site and took a look at the component reports.
Checkstyle alone seems close to pointless as there are over 200 errors in
log4j-api alone. log4j-core has over 2000 errors. Even new files that were
formatted with our formatter settings such as the CassandraAppender plugin
have import ordering errors. I also disagree with some of the rules
configured, but that doesn't really matter when we don't enforce it in the
first place.

Anyways, what's the point of configuring this and adding checkstyle
comments yet not even using it? The only project I've come across in the
wild so far that has checkstyle configured properly was Spring Boot, and
your pull request has to pass the checkstyle check to even be mergeable.

Perhaps if we wish to actually use it, we could loosen the rules down to a
much smaller set that actually matches the formatter settings in src/ide/.
If the rules matched our code base, then we could also have Jenkins run
checkstyle checks which would keep us informed when we mess up, and it
would also be useful for pull requests (I've had to reformat many patches
in the past).

Related, there's the style guide <https://logging.apache.org/
log4j/2.x/javastyle.html> which I'm pretty sure I've never even looked at
before. This could also be normalized with our formatter files. I've
generally thought of our code style summarized as:

* 4 space indent
* use final
* no star imports outside tests (and those should generally be static
imports)
* imports should be in some sort of alphabetical order (this is really
difficult to match between IntelliJ and Eclipse for some reason; I've had
rather obnoxious fights about this in the past thanks to
import-order-induced merge conflicts)
* try to stick to unix line endings, but we're rather mixed still
* every file needs a license header unless it's impossible to include
comments
* use CamelCaseClassNames, even for acronyms
* no hungarian notation or other distracting naming conventions
* otherwise stick to typical Sun style that everyone basically recognizes
(that the JDK doesn't even use itself)

-- 
Matt Sicker <boa...@gmail.com>

Reply via email to