You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by Bessenyei Balázs Donát <be...@apache.org> on 2021/05/18 18:13:14 UTC
Reformat src files with `erlfmt` on `main`
Hi dev@couchdb,
To eliminate the need for formatting-related comments and thus
unnecessary cycles in PRs, I've invested a little time to see if we
could use a formatter on `main` [1].
The PR reformats `.erl` files in `src` and the script [2] included
shows that the compiled binaries match "before" and "after".
The formatter used in the PR is `erlfmt` [3] which is an opinionated
[4] tool so it's more of a "take it or leave it" as-is. (We could try
using other formatters if we want in case people want formatting but
think the choices `erlfmt` makes are unacceptable.)
Some members of the CouchDB dev community already left some great
comments on the PR and I haven't seen any strong opposition so far,
but I wanted to make sure more people are aware of this.
If you have any questions, comments or concerns (or objections),
please let me know.
Thank you,
Donat
[1]: https://github.com/apache/couchdb/pull/3568
[2]: https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
[3]: https://github.com/WhatsApp/erlfmt
[4]: https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
Re: Reformat src files with `erlfmt` on `main`
Posted by Nick Vatamaniuc <va...@gmail.com>.
Good idea, Donát! Thanks for giving it a try.
Even though I personally think emilio has a better style, and I don't
agree with some of the choices erlfmt makes, I still think it would
still be a net benefit to having consistency if we get automated code
reformatting. So +1 overall.
If there are modules or areas we really don't want reformatted, erlfmt
allows comment pragmas (%% erlfmt-ignore [1]) to disable re-formatting
for some sections of code. There are also some suggestions to do minor
manual refactorings which result in a better formatted code [2] , so
we could apply a few of those in some places.
A few more ideas and comments I had were in the PR comment
https://github.com/apache/couchdb/pull/3568#issuecomment-842569320
[1] https://github.com/WhatsApp/erlfmt#ignoring-formatting
[2] https://github.com/WhatsApp/erlfmt#manual-interventions
Cheers,
-Nick
On Tue, May 18, 2021 at 2:13 PM Bessenyei Balázs Donát
<be...@apache.org> wrote:
>
> Hi dev@couchdb,
>
> To eliminate the need for formatting-related comments and thus
> unnecessary cycles in PRs, I've invested a little time to see if we
> could use a formatter on `main` [1].
> The PR reformats `.erl` files in `src` and the script [2] included
> shows that the compiled binaries match "before" and "after".
> The formatter used in the PR is `erlfmt` [3] which is an opinionated
> [4] tool so it's more of a "take it or leave it" as-is. (We could try
> using other formatters if we want in case people want formatting but
> think the choices `erlfmt` makes are unacceptable.)
> Some members of the CouchDB dev community already left some great
> comments on the PR and I haven't seen any strong opposition so far,
> but I wanted to make sure more people are aware of this.
> If you have any questions, comments or concerns (or objections),
> please let me know.
>
>
> Thank you,
>
> Donat
>
>
> [1]: https://github.com/apache/couchdb/pull/3568
> [2]: https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
> [3]: https://github.com/WhatsApp/erlfmt
> [4]: https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
Re: Reformat src files with `erlfmt` on `main`
Posted by Bessenyei Balázs Donát <be...@apache.org>.
Thank you for relaying Paul's approval, @Robert Newson !
> asks if we've confirmed that the abstract syntax tree is unaffected (i.e, the changes are purely cosmetic and make no difference to the compiled artifacts).
The script format_all [1] gets md5sum for the binaries of the files to
be changed and does the same after formatting and reports whether the
hashes match or not. If the script is written correctly, it's proven
that the changes are purely cosmetic and make no difference to the
artifacts.
Donat
[1]: https://github.com/apache/couchdb/pull/3568/files#diff-f2ea7984714072018b8a19daabd2dbc8df9f662af94849ffa7eb60b54bc711f0
On Thu, Jun 3, 2021 at 3:20 PM Robert Newson <rn...@apache.org> wrote:
>
> As Paul's involuntary amanuensis he says "go for it" but asks if we've confirmed that the abstract syntax tree is unaffected (i.e, the changes are purely cosmetic and make no difference to the compiled artifacts).
>
> B.
>
> > On 2 Jun 2021, at 09:18, Bessenyei Balázs Donát <be...@apache.org> wrote:
> >
> >> My only nose-wrinkle was at `->` being placed on its own line under some
> > circumstances
> > I counted too many occurrences of that to add ignores for them (and people
> > would probably forget adding them on new code which would result in a mixed
> > state).
> > If there are no objections, I'll go ahead with merging it with the
> > controversial `->`s on newlines (as advantages seem to outweigh this
> > drawback). As I mentioned earlier, if we can get a config option or change
> > to erlfmt, we can always do a quick reformat.
> >
> >> local git hook
> > I couldn't find a nice way to do it, so I can open a ticket to do that
> > later. The PR adds it to CI and people can run the checks (and the
> > formatter) themselves locally.
> >
> > I have not received a +>=0 from Paul, but as it's been more than a week now
> > I'll merge the PR assuming consent. (The PR is already approved on GitHub.)
> > The change is not irreversible and I'd be happy to either revert or adjust
> > if necessary.
> >
> > Thank you all for the support and the contribution!
> >
> >
> > Donat
> >
> >
> > On Fri, May 28, 2021 at 4:31 PM Ilya Khlopotov <ii...@apache.org> wrote:
> >
> >>> Can it also be set up as a local git hook etc?
> >>
> >> Few complications here:
> >> 1) CouchDB codebase is not 100% resides in a single repository
> >> 2) Which hook manager to use given differences in platforms we support and
> >> the fact that none of the hook managers support multiple repositories.
> >> There are multiple options:
> >>
> >> - https://github.com/frankywahl/super_hooks
> >> - https://github.com/Arkweid/lefthook
> >> - https://github.com/pre-commit/pre-commit
> >>
> >> Do we need a separate ML discussion which hook manager to use?
> >>
> >> Another option is to update configure or rebar.config.script to place
> >> files (or links) in `.git/hooks/pre-commit`.
> >>
> >> Best regards,
> >> iilyak
> >>
> >> On 2021/05/21 12:25:53, Robert Newson <rn...@apache.org> wrote:
> >>> Hi,
> >>>
> >>> My only nose-wrinkle was at `->` being placed on its own line under some
> >> circumstances. The rest looked good. I agree that uniformity of formatting
> >> is a very good thing and this reformat is long overdue.
> >>>
> >>> Agree with Donat that the formatting should be enforced by CI tools so
> >> there’s no backsliding. Can it also be set up as a local git hook etc?
> >>>
> >>> B.
> >>>
> >>>> On 21 May 2021, at 12:46, Bessenyei Balázs Donát <be...@apache.org>
> >> wrote:
> >>>>
> >>>> Hi All,
> >>>>
> >>>> I believe I've only seen +>=0s so far so I intend to (in the following
> >> order):
> >>>> * wait for an ok from @Robert Newson and @Paul J. Davis
> >>>> * add `erlfmt-ignore`s if necessary to #3568
> >>>> * add a check to CI (ideally via `make`) to ensure `erlfmt` is +1 on
> >>>> the PRs in #3568
> >>>> * create a PR for 3.x analogous to #3568
> >>>>
> >>>> Please let me know if I missed anything.
> >>>>
> >>>>
> >>>> Donat
> >>>>
> >>>>
> >>>> On Fri, May 21, 2021 at 2:27 AM Joan Touzet <wo...@apache.org> wrote:
> >>>>>
> >>>>> In general I am +0.5 on the entire thing, but would like to see Bob
> >>>>> Newson or Paul Davis speak up. In the past they've been the most vocal
> >>>>> about code formatting standards, and I'd at least like to see a +0
> >> from
> >>>>> both of them.
> >>>>>
> >>>>> -Joan
> >>>>>
> >>>>> On 20/05/2021 11:53, Ilya Khlopotov wrote:
> >>>>>> Good idea Donat!!!
> >>>>>>
> >>>>>> Even though I disagree with some of the choices made by erlfmt I
> >> appreciate consistency it provides.
> >>>>>> The choices are logical. I really love that every decision is
> >> documented and properly discussed. I did read PR in its entirety and in
> >> fact was not even noticed the ugly `->` in the beginning of the line closer
> >> to the end of the review process.
> >>>>>> I do believe our wetware would adjust in no time to new formatting.
> >> Given how easy it is to reason about. I agree with Donat's observation that
> >> we are spending too much time and emphasis on formatting issues every time
> >> we review PRs. I do believe it is a machine job to provide consistent
> >> formatting. We humans are better at other things. All in all I vote for
> >> adopting `erlfmt` for both 3.x and main.
> >>>>>>
> >>>>>> Also thank you Donat for providing validation scripts to make sure
> >> the re-formatted code compiles to the same beam files.
> >>>>>>
> >>>>>> Best regards,
> >>>>>> iilyak
> >>>>>>
> >>>>>>
> >>>>>> On 2021/05/18 18:13:14, Bessenyei Balázs Donát <be...@apache.org>
> >> wrote:
> >>>>>>> Hi dev@couchdb,
> >>>>>>>
> >>>>>>> To eliminate the need for formatting-related comments and thus
> >>>>>>> unnecessary cycles in PRs, I've invested a little time to see if we
> >>>>>>> could use a formatter on `main` [1].
> >>>>>>> The PR reformats `.erl` files in `src` and the script [2] included
> >>>>>>> shows that the compiled binaries match "before" and "after".
> >>>>>>> The formatter used in the PR is `erlfmt` [3] which is an opinionated
> >>>>>>> [4] tool so it's more of a "take it or leave it" as-is. (We could
> >> try
> >>>>>>> using other formatters if we want in case people want formatting but
> >>>>>>> think the choices `erlfmt` makes are unacceptable.)
> >>>>>>> Some members of the CouchDB dev community already left some great
> >>>>>>> comments on the PR and I haven't seen any strong opposition so far,
> >>>>>>> but I wanted to make sure more people are aware of this.
> >>>>>>> If you have any questions, comments or concerns (or objections),
> >>>>>>> please let me know.
> >>>>>>>
> >>>>>>>
> >>>>>>> Thank you,
> >>>>>>>
> >>>>>>> Donat
> >>>>>>>
> >>>>>>>
> >>>>>>> [1]: https://github.com/apache/couchdb/pull/3568
> >>>>>>> [2]:
> >> https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
> >>>>>>> [3]: https://github.com/WhatsApp/erlfmt
> >>>>>>> [4]:
> >> https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
> >>>>>>>
> >>>
> >>>
> >>
>
Re: Reformat src files with `erlfmt` on `main`
Posted by Robert Newson <rn...@apache.org>.
As Paul's involuntary amanuensis he says "go for it" but asks if we've confirmed that the abstract syntax tree is unaffected (i.e, the changes are purely cosmetic and make no difference to the compiled artifacts).
B.
> On 2 Jun 2021, at 09:18, Bessenyei Balázs Donát <be...@apache.org> wrote:
>
>> My only nose-wrinkle was at `->` being placed on its own line under some
> circumstances
> I counted too many occurrences of that to add ignores for them (and people
> would probably forget adding them on new code which would result in a mixed
> state).
> If there are no objections, I'll go ahead with merging it with the
> controversial `->`s on newlines (as advantages seem to outweigh this
> drawback). As I mentioned earlier, if we can get a config option or change
> to erlfmt, we can always do a quick reformat.
>
>> local git hook
> I couldn't find a nice way to do it, so I can open a ticket to do that
> later. The PR adds it to CI and people can run the checks (and the
> formatter) themselves locally.
>
> I have not received a +>=0 from Paul, but as it's been more than a week now
> I'll merge the PR assuming consent. (The PR is already approved on GitHub.)
> The change is not irreversible and I'd be happy to either revert or adjust
> if necessary.
>
> Thank you all for the support and the contribution!
>
>
> Donat
>
>
> On Fri, May 28, 2021 at 4:31 PM Ilya Khlopotov <ii...@apache.org> wrote:
>
>>> Can it also be set up as a local git hook etc?
>>
>> Few complications here:
>> 1) CouchDB codebase is not 100% resides in a single repository
>> 2) Which hook manager to use given differences in platforms we support and
>> the fact that none of the hook managers support multiple repositories.
>> There are multiple options:
>>
>> - https://github.com/frankywahl/super_hooks
>> - https://github.com/Arkweid/lefthook
>> - https://github.com/pre-commit/pre-commit
>>
>> Do we need a separate ML discussion which hook manager to use?
>>
>> Another option is to update configure or rebar.config.script to place
>> files (or links) in `.git/hooks/pre-commit`.
>>
>> Best regards,
>> iilyak
>>
>> On 2021/05/21 12:25:53, Robert Newson <rn...@apache.org> wrote:
>>> Hi,
>>>
>>> My only nose-wrinkle was at `->` being placed on its own line under some
>> circumstances. The rest looked good. I agree that uniformity of formatting
>> is a very good thing and this reformat is long overdue.
>>>
>>> Agree with Donat that the formatting should be enforced by CI tools so
>> there’s no backsliding. Can it also be set up as a local git hook etc?
>>>
>>> B.
>>>
>>>> On 21 May 2021, at 12:46, Bessenyei Balázs Donát <be...@apache.org>
>> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> I believe I've only seen +>=0s so far so I intend to (in the following
>> order):
>>>> * wait for an ok from @Robert Newson and @Paul J. Davis
>>>> * add `erlfmt-ignore`s if necessary to #3568
>>>> * add a check to CI (ideally via `make`) to ensure `erlfmt` is +1 on
>>>> the PRs in #3568
>>>> * create a PR for 3.x analogous to #3568
>>>>
>>>> Please let me know if I missed anything.
>>>>
>>>>
>>>> Donat
>>>>
>>>>
>>>> On Fri, May 21, 2021 at 2:27 AM Joan Touzet <wo...@apache.org> wrote:
>>>>>
>>>>> In general I am +0.5 on the entire thing, but would like to see Bob
>>>>> Newson or Paul Davis speak up. In the past they've been the most vocal
>>>>> about code formatting standards, and I'd at least like to see a +0
>> from
>>>>> both of them.
>>>>>
>>>>> -Joan
>>>>>
>>>>> On 20/05/2021 11:53, Ilya Khlopotov wrote:
>>>>>> Good idea Donat!!!
>>>>>>
>>>>>> Even though I disagree with some of the choices made by erlfmt I
>> appreciate consistency it provides.
>>>>>> The choices are logical. I really love that every decision is
>> documented and properly discussed. I did read PR in its entirety and in
>> fact was not even noticed the ugly `->` in the beginning of the line closer
>> to the end of the review process.
>>>>>> I do believe our wetware would adjust in no time to new formatting.
>> Given how easy it is to reason about. I agree with Donat's observation that
>> we are spending too much time and emphasis on formatting issues every time
>> we review PRs. I do believe it is a machine job to provide consistent
>> formatting. We humans are better at other things. All in all I vote for
>> adopting `erlfmt` for both 3.x and main.
>>>>>>
>>>>>> Also thank you Donat for providing validation scripts to make sure
>> the re-formatted code compiles to the same beam files.
>>>>>>
>>>>>> Best regards,
>>>>>> iilyak
>>>>>>
>>>>>>
>>>>>> On 2021/05/18 18:13:14, Bessenyei Balázs Donát <be...@apache.org>
>> wrote:
>>>>>>> Hi dev@couchdb,
>>>>>>>
>>>>>>> To eliminate the need for formatting-related comments and thus
>>>>>>> unnecessary cycles in PRs, I've invested a little time to see if we
>>>>>>> could use a formatter on `main` [1].
>>>>>>> The PR reformats `.erl` files in `src` and the script [2] included
>>>>>>> shows that the compiled binaries match "before" and "after".
>>>>>>> The formatter used in the PR is `erlfmt` [3] which is an opinionated
>>>>>>> [4] tool so it's more of a "take it or leave it" as-is. (We could
>> try
>>>>>>> using other formatters if we want in case people want formatting but
>>>>>>> think the choices `erlfmt` makes are unacceptable.)
>>>>>>> Some members of the CouchDB dev community already left some great
>>>>>>> comments on the PR and I haven't seen any strong opposition so far,
>>>>>>> but I wanted to make sure more people are aware of this.
>>>>>>> If you have any questions, comments or concerns (or objections),
>>>>>>> please let me know.
>>>>>>>
>>>>>>>
>>>>>>> Thank you,
>>>>>>>
>>>>>>> Donat
>>>>>>>
>>>>>>>
>>>>>>> [1]: https://github.com/apache/couchdb/pull/3568
>>>>>>> [2]:
>> https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
>>>>>>> [3]: https://github.com/WhatsApp/erlfmt
>>>>>>> [4]:
>> https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
>>>>>>>
>>>
>>>
>>
Re: Reformat src files with `erlfmt` on `main`
Posted by Bessenyei Balázs Donát <be...@apache.org>.
> My only nose-wrinkle was at `->` being placed on its own line under some
circumstances
I counted too many occurrences of that to add ignores for them (and people
would probably forget adding them on new code which would result in a mixed
state).
If there are no objections, I'll go ahead with merging it with the
controversial `->`s on newlines (as advantages seem to outweigh this
drawback). As I mentioned earlier, if we can get a config option or change
to erlfmt, we can always do a quick reformat.
> local git hook
I couldn't find a nice way to do it, so I can open a ticket to do that
later. The PR adds it to CI and people can run the checks (and the
formatter) themselves locally.
I have not received a +>=0 from Paul, but as it's been more than a week now
I'll merge the PR assuming consent. (The PR is already approved on GitHub.)
The change is not irreversible and I'd be happy to either revert or adjust
if necessary.
Thank you all for the support and the contribution!
Donat
On Fri, May 28, 2021 at 4:31 PM Ilya Khlopotov <ii...@apache.org> wrote:
> > Can it also be set up as a local git hook etc?
>
> Few complications here:
> 1) CouchDB codebase is not 100% resides in a single repository
> 2) Which hook manager to use given differences in platforms we support and
> the fact that none of the hook managers support multiple repositories.
> There are multiple options:
>
> - https://github.com/frankywahl/super_hooks
> - https://github.com/Arkweid/lefthook
> - https://github.com/pre-commit/pre-commit
>
> Do we need a separate ML discussion which hook manager to use?
>
> Another option is to update configure or rebar.config.script to place
> files (or links) in `.git/hooks/pre-commit`.
>
> Best regards,
> iilyak
>
> On 2021/05/21 12:25:53, Robert Newson <rn...@apache.org> wrote:
> > Hi,
> >
> > My only nose-wrinkle was at `->` being placed on its own line under some
> circumstances. The rest looked good. I agree that uniformity of formatting
> is a very good thing and this reformat is long overdue.
> >
> > Agree with Donat that the formatting should be enforced by CI tools so
> there’s no backsliding. Can it also be set up as a local git hook etc?
> >
> > B.
> >
> > > On 21 May 2021, at 12:46, Bessenyei Balázs Donát <be...@apache.org>
> wrote:
> > >
> > > Hi All,
> > >
> > > I believe I've only seen +>=0s so far so I intend to (in the following
> order):
> > > * wait for an ok from @Robert Newson and @Paul J. Davis
> > > * add `erlfmt-ignore`s if necessary to #3568
> > > * add a check to CI (ideally via `make`) to ensure `erlfmt` is +1 on
> > > the PRs in #3568
> > > * create a PR for 3.x analogous to #3568
> > >
> > > Please let me know if I missed anything.
> > >
> > >
> > > Donat
> > >
> > >
> > > On Fri, May 21, 2021 at 2:27 AM Joan Touzet <wo...@apache.org> wrote:
> > >>
> > >> In general I am +0.5 on the entire thing, but would like to see Bob
> > >> Newson or Paul Davis speak up. In the past they've been the most vocal
> > >> about code formatting standards, and I'd at least like to see a +0
> from
> > >> both of them.
> > >>
> > >> -Joan
> > >>
> > >> On 20/05/2021 11:53, Ilya Khlopotov wrote:
> > >>> Good idea Donat!!!
> > >>>
> > >>> Even though I disagree with some of the choices made by erlfmt I
> appreciate consistency it provides.
> > >>> The choices are logical. I really love that every decision is
> documented and properly discussed. I did read PR in its entirety and in
> fact was not even noticed the ugly `->` in the beginning of the line closer
> to the end of the review process.
> > >>> I do believe our wetware would adjust in no time to new formatting.
> Given how easy it is to reason about. I agree with Donat's observation that
> we are spending too much time and emphasis on formatting issues every time
> we review PRs. I do believe it is a machine job to provide consistent
> formatting. We humans are better at other things. All in all I vote for
> adopting `erlfmt` for both 3.x and main.
> > >>>
> > >>> Also thank you Donat for providing validation scripts to make sure
> the re-formatted code compiles to the same beam files.
> > >>>
> > >>> Best regards,
> > >>> iilyak
> > >>>
> > >>>
> > >>> On 2021/05/18 18:13:14, Bessenyei Balázs Donát <be...@apache.org>
> wrote:
> > >>>> Hi dev@couchdb,
> > >>>>
> > >>>> To eliminate the need for formatting-related comments and thus
> > >>>> unnecessary cycles in PRs, I've invested a little time to see if we
> > >>>> could use a formatter on `main` [1].
> > >>>> The PR reformats `.erl` files in `src` and the script [2] included
> > >>>> shows that the compiled binaries match "before" and "after".
> > >>>> The formatter used in the PR is `erlfmt` [3] which is an opinionated
> > >>>> [4] tool so it's more of a "take it or leave it" as-is. (We could
> try
> > >>>> using other formatters if we want in case people want formatting but
> > >>>> think the choices `erlfmt` makes are unacceptable.)
> > >>>> Some members of the CouchDB dev community already left some great
> > >>>> comments on the PR and I haven't seen any strong opposition so far,
> > >>>> but I wanted to make sure more people are aware of this.
> > >>>> If you have any questions, comments or concerns (or objections),
> > >>>> please let me know.
> > >>>>
> > >>>>
> > >>>> Thank you,
> > >>>>
> > >>>> Donat
> > >>>>
> > >>>>
> > >>>> [1]: https://github.com/apache/couchdb/pull/3568
> > >>>> [2]:
> https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
> > >>>> [3]: https://github.com/WhatsApp/erlfmt
> > >>>> [4]:
> https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
> > >>>>
> >
> >
>
Re: Reformat src files with `erlfmt` on `main`
Posted by Ilya Khlopotov <ii...@apache.org>.
> Can it also be set up as a local git hook etc?
Few complications here:
1) CouchDB codebase is not 100% resides in a single repository
2) Which hook manager to use given differences in platforms we support and the fact that none of the hook managers support multiple repositories. There are multiple options:
- https://github.com/frankywahl/super_hooks
- https://github.com/Arkweid/lefthook
- https://github.com/pre-commit/pre-commit
Do we need a separate ML discussion which hook manager to use?
Another option is to update configure or rebar.config.script to place files (or links) in `.git/hooks/pre-commit`.
Best regards,
iilyak
On 2021/05/21 12:25:53, Robert Newson <rn...@apache.org> wrote:
> Hi,
>
> My only nose-wrinkle was at `->` being placed on its own line under some circumstances. The rest looked good. I agree that uniformity of formatting is a very good thing and this reformat is long overdue.
>
> Agree with Donat that the formatting should be enforced by CI tools so there’s no backsliding. Can it also be set up as a local git hook etc?
>
> B.
>
> > On 21 May 2021, at 12:46, Bessenyei Balázs Donát <be...@apache.org> wrote:
> >
> > Hi All,
> >
> > I believe I've only seen +>=0s so far so I intend to (in the following order):
> > * wait for an ok from @Robert Newson and @Paul J. Davis
> > * add `erlfmt-ignore`s if necessary to #3568
> > * add a check to CI (ideally via `make`) to ensure `erlfmt` is +1 on
> > the PRs in #3568
> > * create a PR for 3.x analogous to #3568
> >
> > Please let me know if I missed anything.
> >
> >
> > Donat
> >
> >
> > On Fri, May 21, 2021 at 2:27 AM Joan Touzet <wo...@apache.org> wrote:
> >>
> >> In general I am +0.5 on the entire thing, but would like to see Bob
> >> Newson or Paul Davis speak up. In the past they've been the most vocal
> >> about code formatting standards, and I'd at least like to see a +0 from
> >> both of them.
> >>
> >> -Joan
> >>
> >> On 20/05/2021 11:53, Ilya Khlopotov wrote:
> >>> Good idea Donat!!!
> >>>
> >>> Even though I disagree with some of the choices made by erlfmt I appreciate consistency it provides.
> >>> The choices are logical. I really love that every decision is documented and properly discussed. I did read PR in its entirety and in fact was not even noticed the ugly `->` in the beginning of the line closer to the end of the review process.
> >>> I do believe our wetware would adjust in no time to new formatting. Given how easy it is to reason about. I agree with Donat's observation that we are spending too much time and emphasis on formatting issues every time we review PRs. I do believe it is a machine job to provide consistent formatting. We humans are better at other things. All in all I vote for adopting `erlfmt` for both 3.x and main.
> >>>
> >>> Also thank you Donat for providing validation scripts to make sure the re-formatted code compiles to the same beam files.
> >>>
> >>> Best regards,
> >>> iilyak
> >>>
> >>>
> >>> On 2021/05/18 18:13:14, Bessenyei Balázs Donát <be...@apache.org> wrote:
> >>>> Hi dev@couchdb,
> >>>>
> >>>> To eliminate the need for formatting-related comments and thus
> >>>> unnecessary cycles in PRs, I've invested a little time to see if we
> >>>> could use a formatter on `main` [1].
> >>>> The PR reformats `.erl` files in `src` and the script [2] included
> >>>> shows that the compiled binaries match "before" and "after".
> >>>> The formatter used in the PR is `erlfmt` [3] which is an opinionated
> >>>> [4] tool so it's more of a "take it or leave it" as-is. (We could try
> >>>> using other formatters if we want in case people want formatting but
> >>>> think the choices `erlfmt` makes are unacceptable.)
> >>>> Some members of the CouchDB dev community already left some great
> >>>> comments on the PR and I haven't seen any strong opposition so far,
> >>>> but I wanted to make sure more people are aware of this.
> >>>> If you have any questions, comments or concerns (or objections),
> >>>> please let me know.
> >>>>
> >>>>
> >>>> Thank you,
> >>>>
> >>>> Donat
> >>>>
> >>>>
> >>>> [1]: https://github.com/apache/couchdb/pull/3568
> >>>> [2]: https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
> >>>> [3]: https://github.com/WhatsApp/erlfmt
> >>>> [4]: https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
> >>>>
>
>
Re: Reformat src files with `erlfmt` on `main`
Posted by Robert Newson <rn...@apache.org>.
Hi,
My only nose-wrinkle was at `->` being placed on its own line under some circumstances. The rest looked good. I agree that uniformity of formatting is a very good thing and this reformat is long overdue.
Agree with Donat that the formatting should be enforced by CI tools so there’s no backsliding. Can it also be set up as a local git hook etc?
B.
> On 21 May 2021, at 12:46, Bessenyei Balázs Donát <be...@apache.org> wrote:
>
> Hi All,
>
> I believe I've only seen +>=0s so far so I intend to (in the following order):
> * wait for an ok from @Robert Newson and @Paul J. Davis
> * add `erlfmt-ignore`s if necessary to #3568
> * add a check to CI (ideally via `make`) to ensure `erlfmt` is +1 on
> the PRs in #3568
> * create a PR for 3.x analogous to #3568
>
> Please let me know if I missed anything.
>
>
> Donat
>
>
> On Fri, May 21, 2021 at 2:27 AM Joan Touzet <wo...@apache.org> wrote:
>>
>> In general I am +0.5 on the entire thing, but would like to see Bob
>> Newson or Paul Davis speak up. In the past they've been the most vocal
>> about code formatting standards, and I'd at least like to see a +0 from
>> both of them.
>>
>> -Joan
>>
>> On 20/05/2021 11:53, Ilya Khlopotov wrote:
>>> Good idea Donat!!!
>>>
>>> Even though I disagree with some of the choices made by erlfmt I appreciate consistency it provides.
>>> The choices are logical. I really love that every decision is documented and properly discussed. I did read PR in its entirety and in fact was not even noticed the ugly `->` in the beginning of the line closer to the end of the review process.
>>> I do believe our wetware would adjust in no time to new formatting. Given how easy it is to reason about. I agree with Donat's observation that we are spending too much time and emphasis on formatting issues every time we review PRs. I do believe it is a machine job to provide consistent formatting. We humans are better at other things. All in all I vote for adopting `erlfmt` for both 3.x and main.
>>>
>>> Also thank you Donat for providing validation scripts to make sure the re-formatted code compiles to the same beam files.
>>>
>>> Best regards,
>>> iilyak
>>>
>>>
>>> On 2021/05/18 18:13:14, Bessenyei Balázs Donát <be...@apache.org> wrote:
>>>> Hi dev@couchdb,
>>>>
>>>> To eliminate the need for formatting-related comments and thus
>>>> unnecessary cycles in PRs, I've invested a little time to see if we
>>>> could use a formatter on `main` [1].
>>>> The PR reformats `.erl` files in `src` and the script [2] included
>>>> shows that the compiled binaries match "before" and "after".
>>>> The formatter used in the PR is `erlfmt` [3] which is an opinionated
>>>> [4] tool so it's more of a "take it or leave it" as-is. (We could try
>>>> using other formatters if we want in case people want formatting but
>>>> think the choices `erlfmt` makes are unacceptable.)
>>>> Some members of the CouchDB dev community already left some great
>>>> comments on the PR and I haven't seen any strong opposition so far,
>>>> but I wanted to make sure more people are aware of this.
>>>> If you have any questions, comments or concerns (or objections),
>>>> please let me know.
>>>>
>>>>
>>>> Thank you,
>>>>
>>>> Donat
>>>>
>>>>
>>>> [1]: https://github.com/apache/couchdb/pull/3568
>>>> [2]: https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
>>>> [3]: https://github.com/WhatsApp/erlfmt
>>>> [4]: https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
>>>>
Re: Reformat src files with `erlfmt` on `main`
Posted by Bessenyei Balázs Donát <be...@apache.org>.
Hi All,
I believe I've only seen +>=0s so far so I intend to (in the following order):
* wait for an ok from @Robert Newson and @Paul J. Davis
* add `erlfmt-ignore`s if necessary to #3568
* add a check to CI (ideally via `make`) to ensure `erlfmt` is +1 on
the PRs in #3568
* create a PR for 3.x analogous to #3568
Please let me know if I missed anything.
Donat
On Fri, May 21, 2021 at 2:27 AM Joan Touzet <wo...@apache.org> wrote:
>
> In general I am +0.5 on the entire thing, but would like to see Bob
> Newson or Paul Davis speak up. In the past they've been the most vocal
> about code formatting standards, and I'd at least like to see a +0 from
> both of them.
>
> -Joan
>
> On 20/05/2021 11:53, Ilya Khlopotov wrote:
> > Good idea Donat!!!
> >
> > Even though I disagree with some of the choices made by erlfmt I appreciate consistency it provides.
> > The choices are logical. I really love that every decision is documented and properly discussed. I did read PR in its entirety and in fact was not even noticed the ugly `->` in the beginning of the line closer to the end of the review process.
> > I do believe our wetware would adjust in no time to new formatting. Given how easy it is to reason about. I agree with Donat's observation that we are spending too much time and emphasis on formatting issues every time we review PRs. I do believe it is a machine job to provide consistent formatting. We humans are better at other things. All in all I vote for adopting `erlfmt` for both 3.x and main.
> >
> > Also thank you Donat for providing validation scripts to make sure the re-formatted code compiles to the same beam files.
> >
> > Best regards,
> > iilyak
> >
> >
> > On 2021/05/18 18:13:14, Bessenyei Balázs Donát <be...@apache.org> wrote:
> >> Hi dev@couchdb,
> >>
> >> To eliminate the need for formatting-related comments and thus
> >> unnecessary cycles in PRs, I've invested a little time to see if we
> >> could use a formatter on `main` [1].
> >> The PR reformats `.erl` files in `src` and the script [2] included
> >> shows that the compiled binaries match "before" and "after".
> >> The formatter used in the PR is `erlfmt` [3] which is an opinionated
> >> [4] tool so it's more of a "take it or leave it" as-is. (We could try
> >> using other formatters if we want in case people want formatting but
> >> think the choices `erlfmt` makes are unacceptable.)
> >> Some members of the CouchDB dev community already left some great
> >> comments on the PR and I haven't seen any strong opposition so far,
> >> but I wanted to make sure more people are aware of this.
> >> If you have any questions, comments or concerns (or objections),
> >> please let me know.
> >>
> >>
> >> Thank you,
> >>
> >> Donat
> >>
> >>
> >> [1]: https://github.com/apache/couchdb/pull/3568
> >> [2]: https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
> >> [3]: https://github.com/WhatsApp/erlfmt
> >> [4]: https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
> >>
Re: Reformat src files with `erlfmt` on `main`
Posted by Joan Touzet <wo...@apache.org>.
In general I am +0.5 on the entire thing, but would like to see Bob
Newson or Paul Davis speak up. In the past they've been the most vocal
about code formatting standards, and I'd at least like to see a +0 from
both of them.
-Joan
On 20/05/2021 11:53, Ilya Khlopotov wrote:
> Good idea Donat!!!
>
> Even though I disagree with some of the choices made by erlfmt I appreciate consistency it provides.
> The choices are logical. I really love that every decision is documented and properly discussed. I did read PR in its entirety and in fact was not even noticed the ugly `->` in the beginning of the line closer to the end of the review process.
> I do believe our wetware would adjust in no time to new formatting. Given how easy it is to reason about. I agree with Donat's observation that we are spending too much time and emphasis on formatting issues every time we review PRs. I do believe it is a machine job to provide consistent formatting. We humans are better at other things. All in all I vote for adopting `erlfmt` for both 3.x and main.
>
> Also thank you Donat for providing validation scripts to make sure the re-formatted code compiles to the same beam files.
>
> Best regards,
> iilyak
>
>
> On 2021/05/18 18:13:14, Bessenyei Balázs Donát <be...@apache.org> wrote:
>> Hi dev@couchdb,
>>
>> To eliminate the need for formatting-related comments and thus
>> unnecessary cycles in PRs, I've invested a little time to see if we
>> could use a formatter on `main` [1].
>> The PR reformats `.erl` files in `src` and the script [2] included
>> shows that the compiled binaries match "before" and "after".
>> The formatter used in the PR is `erlfmt` [3] which is an opinionated
>> [4] tool so it's more of a "take it or leave it" as-is. (We could try
>> using other formatters if we want in case people want formatting but
>> think the choices `erlfmt` makes are unacceptable.)
>> Some members of the CouchDB dev community already left some great
>> comments on the PR and I haven't seen any strong opposition so far,
>> but I wanted to make sure more people are aware of this.
>> If you have any questions, comments or concerns (or objections),
>> please let me know.
>>
>>
>> Thank you,
>>
>> Donat
>>
>>
>> [1]: https://github.com/apache/couchdb/pull/3568
>> [2]: https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
>> [3]: https://github.com/WhatsApp/erlfmt
>> [4]: https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
>>
Re: Reformat src files with `erlfmt` on `main`
Posted by Ilya Khlopotov <ii...@apache.org>.
Good idea Donat!!!
Even though I disagree with some of the choices made by erlfmt I appreciate consistency it provides.
The choices are logical. I really love that every decision is documented and properly discussed. I did read PR in its entirety and in fact was not even noticed the ugly `->` in the beginning of the line closer to the end of the review process.
I do believe our wetware would adjust in no time to new formatting. Given how easy it is to reason about. I agree with Donat's observation that we are spending too much time and emphasis on formatting issues every time we review PRs. I do believe it is a machine job to provide consistent formatting. We humans are better at other things. All in all I vote for adopting `erlfmt` for both 3.x and main.
Also thank you Donat for providing validation scripts to make sure the re-formatted code compiles to the same beam files.
Best regards,
iilyak
On 2021/05/18 18:13:14, Bessenyei Balázs Donát <be...@apache.org> wrote:
> Hi dev@couchdb,
>
> To eliminate the need for formatting-related comments and thus
> unnecessary cycles in PRs, I've invested a little time to see if we
> could use a formatter on `main` [1].
> The PR reformats `.erl` files in `src` and the script [2] included
> shows that the compiled binaries match "before" and "after".
> The formatter used in the PR is `erlfmt` [3] which is an opinionated
> [4] tool so it's more of a "take it or leave it" as-is. (We could try
> using other formatters if we want in case people want formatting but
> think the choices `erlfmt` makes are unacceptable.)
> Some members of the CouchDB dev community already left some great
> comments on the PR and I haven't seen any strong opposition so far,
> but I wanted to make sure more people are aware of this.
> If you have any questions, comments or concerns (or objections),
> please let me know.
>
>
> Thank you,
>
> Donat
>
>
> [1]: https://github.com/apache/couchdb/pull/3568
> [2]: https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49ff2b760b81c006097f20f412ea2007f899042fd0de98a
> [3]: https://github.com/WhatsApp/erlfmt
> [4]: https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters
>