> 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 <iil...@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 <rnew...@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 <bes...@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 <woh...@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 <bes...@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 > > >>>> > > > > >
