From dev-return-49874-archive-asf-public=cust-asf.ponee.io@couchdb.apache.org Wed Jun 2 08:18:27 2021 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mxout1-ec2-va.apache.org (mxout1-ec2-va.apache.org [3.227.148.255]) by mx-eu-01.ponee.io (Postfix) with ESMTPS id 5A149180638 for ; Wed, 2 Jun 2021 10:18:27 +0200 (CEST) Received: from mail.apache.org (mailroute1-lw-us.apache.org [207.244.88.153]) by mxout1-ec2-va.apache.org (ASF Mail Server at mxout1-ec2-va.apache.org) with SMTP id 53517403D1 for ; Wed, 2 Jun 2021 08:18:26 +0000 (UTC) Received: (qmail 49241 invoked by uid 500); 2 Jun 2021 08:18:25 -0000 Mailing-List: contact dev-help@couchdb.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@couchdb.apache.org Delivered-To: mailing list dev@couchdb.apache.org Received: (qmail 49229 invoked by uid 99); 2 Jun 2021 08:18:25 -0000 Received: from mailrelay1-he-de.apache.org (HELO mailrelay1-he-de.apache.org) (116.203.21.61) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 02 Jun 2021 08:18:25 +0000 Received: from mail-io1-f53.google.com (mail-io1-f53.google.com [209.85.166.53]) by mailrelay1-he-de.apache.org (ASF Mail Server at mailrelay1-he-de.apache.org) with ESMTPSA id 603343E823 for ; Wed, 2 Jun 2021 08:18:24 +0000 (UTC) Received: by mail-io1-f53.google.com with SMTP id a8so1566600ioa.12 for ; Wed, 02 Jun 2021 01:18:24 -0700 (PDT) X-Gm-Message-State: AOAM530JtjY2UX+aHmXcBzF1fKKwjBh5+SAt+rJ0zRUuZa1Mung8qlUX c0Mi9nuYtysvcl0D6mtX0oGP/UFSmtGsrHfM9YA= X-Google-Smtp-Source: ABdhPJwNmYHVGXwPmcCRnufeUHv0OslyeMvPENxDMSd+Yde/NqVtvw2/tRjNErweQ19xzMTnBbKlPHtUv/zYDsSepUs= X-Received: by 2002:a02:c73a:: with SMTP id h26mr29269818jao.95.1622621903272; Wed, 02 Jun 2021 01:18:23 -0700 (PDT) MIME-Version: 1.0 References: <1958d593-f0b2-f2c0-3ce3-8e3356159263@apache.org> In-Reply-To: From: =?UTF-8?B?QmVzc2VueWVpIEJhbMOhenMgRG9uw6F0?= Date: Wed, 2 Jun 2021 10:18:12 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: Reformat src files with `erlfmt` on `main` To: dev@couchdb.apache.org Content-Type: multipart/alternative; boundary="00000000000026407b05c3c41a2e" --00000000000026407b05c3c41a2e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > 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 +>=3D0 from Paul, but as it's been more than a week n= ow 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 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 an= d > 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 wrote: > > Hi, > > > > My only nose-wrinkle was at `->` being placed on its own line under som= e > circumstances. The rest looked good. I agree that uniformity of formattin= g > 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=E2=80=99s no backsliding. Can it also be set up as a local git hook= etc? > > > > B. > > > > > On 21 May 2021, at 12:46, Bessenyei Bal=C3=A1zs Don=C3=A1t > wrote: > > > > > > Hi All, > > > > > > I believe I've only seen +>=3D0s so far so I intend to (in the follow= ing > 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 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 voc= al > > >> 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 clos= er > 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 th= at > we are spending too much time and emphasis on formatting issues every tim= e > 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=C3=A1zs Don=C3=A1t > 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 w= e > > >>>> 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 opinionat= ed > > >>>> [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 b= ut > > >>>> 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-7adfbc2d8dba4d4ff4= 9ff2b760b81c006097f20f412ea2007f899042fd0de98a > > >>>> [3]: https://github.com/WhatsApp/erlfmt > > >>>> [4]: > https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatter= s > > >>>> > > > > > --00000000000026407b05c3c41a2e--