Good points, Dan. Checkstyle will still run, but just focused on the things that go beyond format.
Kenn On Wed, Jun 27, 2018 at 8:03 AM Etienne Chauchot <echauc...@apache.org> wrote: > +1 ! > It's my custom to avoid reformatting to spare meaningless diff burden to > the reviewer. Now it will be over, thanks. > > Etienne > > Le mardi 26 juin 2018 à 21:15 -0700, Kenneth Knowles a écrit : > > Hi all, > > I like readable code, but I don't like formatting it myself. And I > _really_ don't like discussing in code review. "Spotless" [1] can enforce - > and automatically apply - automatic formatting for Java, Groovy, and some > others. > > This is not about style or wanting a particular layout. This is about > automation, contributor experience, and streamlining review > > - Contributor experience: MUCH better than checkstyle: error message just > says "run ./gradlew :beam-your-module:spotlessApply" instead of telling > them to go in and manually edit. > > - Automation: You want to use autoformat so you don't have to format code > by hand. But if you autoformat a file that was in some other format, then > you touch a bunch of unrelated lines. If the file is already autoformatted, > it is much better. > > - Review: Never talk about code formatting ever again. A PR also needs > baseline to already be autoformatted or formatting will make it unclear > which lines are really changed. > > This is already available via applyJavaNature(enableSpotless: true) and it > is turned on for SQL and our buildSrc gradle plugins. It is very nice. > There is a JIRA [2] to turn it on for the hold code base. Personally, I > think (a) every module could make a different choice if the main > contributors feel strongly and (b) it is objectively better to always > autoformat :-) > > WDYT? If we do it, it is trivial to add it module-at-a-time or globally. > If someone conflicts with a massive autoformat commit, they can just keep > their changes and autoformat them and it is done. > > Kenn > > [1] https://github.com/diffplug/spotless/tree/master/plugin-gradle > [2] https://issues.apache.org/jira/browse/BEAM-4394 > >
