couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ilya Khlopotov <iil...@apache.org>
Subject Re: Reformat src files with `erlfmt` on `main`
Date Fri, 28 May 2021 14:31:23 GMT
> 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 <rnewson@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 <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