Luke: Anything that helps contributors and reviewers work better together - +1! :D
On Wed, Jun 27, 2018 at 9:04 AM Lukasz Cwik <lc...@google.com> wrote: > If spotless is run against a PR that is already well formatted its a > non-issue as the formatting changes are usually related to the change but I > have reviewed a few PRs that have 100s of lines of formatting change which > really obfuscates the work. > Instead of asking contributors to run spotless, can we have a cron job run > it across the project like once a day/week/... and cut a PR? > > On Wed, Jun 27, 2018 at 8:07 AM Kenneth Knowles <k...@google.com> wrote: > >> 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 >>> >>>
smime.p7s
Description: S/MIME Cryptographic Signature
