From dev-return-49875-archive-asf-public=cust-asf.ponee.io@couchdb.apache.org Thu Jun 3 13:20:20 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-he-de.apache.org (mxout1-he-de.apache.org [95.216.194.37]) by mx-eu-01.ponee.io (Postfix) with ESMTPS id 42BDD180643 for ; Thu, 3 Jun 2021 15:20:20 +0200 (CEST) Received: from mail.apache.org (mailroute1-lw-us.apache.org [207.244.88.153]) by mxout1-he-de.apache.org (ASF Mail Server at mxout1-he-de.apache.org) with SMTP id 36EE461FEF for ; Thu, 3 Jun 2021 13:20:09 +0000 (UTC) Received: (qmail 53636 invoked by uid 500); 3 Jun 2021 13:20:05 -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 53478 invoked by uid 99); 3 Jun 2021 13:20:04 -0000 Received: from ec2-52-204-25-47.compute-1.amazonaws.com (HELO mailrelay1-ec2-va.apache.org) (52.204.25.47) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 03 Jun 2021 13:20:04 +0000 Received: from auth2-smtp.messagingengine.com (auth2-smtp.messagingengine.com [66.111.4.228]) by mailrelay1-ec2-va.apache.org (ASF Mail Server at mailrelay1-ec2-va.apache.org) with ESMTPSA id 2889D41A80 for ; Thu, 3 Jun 2021 13:20:04 +0000 (UTC) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailauth.nyi.internal (Postfix) with ESMTP id 04CC627C005A for ; Thu, 3 Jun 2021 09:20:03 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Thu, 03 Jun 2021 09:20:04 -0400 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrvdelledgiedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpefhtgfgggfuffhfvfgjkffosehtqh hmtdhhtdejnecuhfhrohhmpeftohgsvghrthcupfgvfihsohhnuceorhhnvgifshhonhes rghprggthhgvrdhorhhgqeenucggtffrrghtthgvrhhnpefftdeujeejteelhfelieelff egudekffeiheehheffudelgfdtkedvhfeukefggeenucffohhmrghinhepghhithhhuhgs rdgtohhmnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomh eprhhnvgifshhonhdomhgvshhmthhprghuthhhphgvrhhsohhnrghlihhthidqleefgedv tddvjedvqdduudelgeejtdejjedqrhhnvgifshhonheppegrphgrtghhvgdrohhrghesfh grshhtmhgrihhlrdhfmh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Thu, 3 Jun 2021 09:20:03 -0400 (EDT) From: Robert Newson Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.80.0.2.43\)) Subject: Re: Reformat src files with `erlfmt` on `main` Date: Thu, 3 Jun 2021 14:20:01 +0100 References: <1958d593-f0b2-f2c0-3ce3-8e3356159263@apache.org> To: CouchDB Developers In-Reply-To: Message-Id: X-Mailer: Apple Mail (2.3654.80.0.2.43) 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=C3=A1zs Don=C3=A1t = wrote: >=20 >> 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. >=20 >> 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. >=20 > I have not received a +>=3D0 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. >=20 > Thank you all for the support and the contribution! >=20 >=20 > Donat >=20 >=20 > On Fri, May 28, 2021 at 4:31 PM Ilya Khlopotov = wrote: >=20 >>> Can it also be set up as a local git hook etc? >>=20 >> 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: >>=20 >> - https://github.com/frankywahl/super_hooks >> - https://github.com/Arkweid/lefthook >> - https://github.com/pre-commit/pre-commit >>=20 >> Do we need a separate ML discussion which hook manager to use? >>=20 >> Another option is to update configure or rebar.config.script to place >> files (or links) in `.git/hooks/pre-commit`. >>=20 >> Best regards, >> iilyak >>=20 >> On 2021/05/21 12:25:53, Robert Newson wrote: >>> Hi, >>>=20 >>> 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. >>>=20 >>> 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? >>>=20 >>> B. >>>=20 >>>> On 21 May 2021, at 12:46, Bessenyei Bal=C3=A1zs Don=C3=A1t = >> wrote: >>>>=20 >>>> Hi All, >>>>=20 >>>> I believe I've only seen +>=3D0s 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 >>>>=20 >>>> Please let me know if I missed anything. >>>>=20 >>>>=20 >>>> Donat >>>>=20 >>>>=20 >>>> On Fri, May 21, 2021 at 2:27 AM Joan Touzet = wrote: >>>>>=20 >>>>> 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. >>>>>=20 >>>>> -Joan >>>>>=20 >>>>> On 20/05/2021 11:53, Ilya Khlopotov wrote: >>>>>> Good idea Donat!!! >>>>>>=20 >>>>>> 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. >>>>>>=20 >>>>>> Also thank you Donat for providing validation scripts to make = sure >> the re-formatted code compiles to the same beam files. >>>>>>=20 >>>>>> Best regards, >>>>>> iilyak >>>>>>=20 >>>>>>=20 >>>>>> On 2021/05/18 18:13:14, Bessenyei Bal=C3=A1zs Don=C3=A1t = >> wrote: >>>>>>> Hi dev@couchdb, >>>>>>>=20 >>>>>>> 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. >>>>>>>=20 >>>>>>>=20 >>>>>>> Thank you, >>>>>>>=20 >>>>>>> Donat >>>>>>>=20 >>>>>>>=20 >>>>>>> [1]: https://github.com/apache/couchdb/pull/3568 >>>>>>> [2]: >> = https://github.com/apache/couchdb/pull/3568/files#diff-7adfbc2d8dba4d4ff49= ff2b760b81c006097f20f412ea2007f899042fd0de98a >>>>>>> [3]: https://github.com/WhatsApp/erlfmt >>>>>>> [4]: >> = https://github.com/WhatsApp/erlfmt#comparison-with-other-erlang-formatters= >>>>>>>=20 >>>=20 >>>=20 >>=20