couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Newson <rnew...@apache.org>
Subject Re: Reformat src files with `erlfmt` on `main`
Date Fri, 21 May 2021 12:25:53 GMT
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 <bessbd@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 <wohali@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 <bessbd@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
>>>> 


Mime
View raw message