Github user ctubbsii commented on the pull request:
https://github.com/apache/accumulo/pull/35#issuecomment-99120050
There are two main problems with the Eclipse formatter:
1. It doesn't strictly adhere to the line length restrictions.
2. It adds a trailing space to indent empty javadoc lines.
We've already accounted for the line length in the checkstyle rules by
making it much larger than the Eclipse formatter. With this patch, I think we
could probably just ignore trailing whitespace (and drop the corresponding
checkstyle enforcement), since the formatter will remove the ones we mainly
care about anyway (the javadoc ones are annoying, but there's not a lot of good
options so long as Eclipse fails to fix that issue). We could just strip
whitespace with `sed` after the formatter executes.
One thing I'd like to see, though (for the purposes of a review) is just
the pom changes, separate from any resulting formatting changes (I'm sure I can
do this with some command-line Git Fu, though, so it's no big deal).
I did see that the formatting significantly changed some lines that looked
deliberately formatted to make it more readable. For those lines, we may wish
to add the `@formatter:off` / `@formatter:on` tags to retain existing
formatting.
I'm also curious what happens with the `-Pthrift` profile activated. Will
this plugin reformat the generated thrift code? Maybe that's okay, but it might
be annoying. If the generated thrift code were in a separate package, it could
be avoided more easily, though.
I'm overall +1 to the idea, but it might need some tweaking.
I especially like the idea that we could focus our style choices on
readability, because writing to conform to a particular style doesn't matter
any more (write in whatever style you like and the build will fix it).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
