+1.
Wondering if it can be configured to reformat only what we care most about
(2 space indentation etc), allowing some discretion on the edges. An
example of inconsistent formatting that ends up in my code:
---
anObject.someLongMethodName(arg_number_1,
arg_number_2);
--- vs ---
anObject.anotherMethodName(
arg_number_1,
arg_number_2
);
On Wed, Jun 27, 2018 at 9:41 AM Lukasz Cwik <lc...@google.com> wrote:
> It wasn't clear to me that the intent was to autoformat all the code from
> the proposal initially. If thats the case, then the delta is quite small
> typically.
>
> Also, it would be easier if we recommended to users to run run "./gradlew
> spotlessApply"
> which will run spotless on all modules.
>
> On Wed, Jun 27, 2018 at 9:31 AM Kenneth Knowles <k...@google.com> wrote:
>
>> Luke: the proposal here solves exactly what you are talking about.
>>
>> The problem you describe happens when the PR author uses autoformat but
>> the baseline is not already autoformatted. What I am proposing is to make
>> sure the baseline is already autoformatted, so PRs never have extraneous
>> formatting changes.
>>
>> Rafael: the default setting on GitHub is "allow edits by maintainers" so
>> actually a committer can run spotless on behalf of a contributor and push
>> the fixup. I have done this. It also lets a committer fix up a good PR
>> and merge it even if the contributor is, say, asleep.
>>
>> Kenn
>>
>> On Wed, Jun 27, 2018 at 9:24 AM Rafael Fernandez <rfern...@google.com>
>> wrote:
>>
>>> 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
>>>>>>
>>>>>>
