[GitHub] [couchdb] nickva commented on pull request #3568: Reformat src files with erlfmt

[GitBox <gi...@apache.org>]

nickva commented on pull request #3568:
URL: https://github.com/apache/couchdb/pull/3568#issuecomment-842569320


   Nice work @bessbd. 
   
    * I like that it is consistent and it auto-formats the code that is a big plus and it removes a good amount of back-and-forth during reviews
    
    * Good call to check the hashes before and after, that's a neat trick!
    
    * Some erlfmt choices are odd, such as putting `->` on a line by itself. I don't like it but not a deal breaker compared to other benefits
   
    * In general I would prefer 80 columns (that is configurable `erlfmt --print-width=80`) but if most people would want to stick with the default 100 that's ok too. It seems, based on how case nesting is reformatted (always indented with 4 spaces) some places like auth handlers end up being squished to the right even with 100 columns. With 80 it would be worse so I can live with 100 columns.
   
    * I don't like the spacing between functions as much, I prefer one line between function head clauses, and two lines between functions. However, if erlfmt means having consistency I am ok sticking with erlfmt's way.
   
   Based on @iilyak's scale (-5...+2), I'd go with +1 as an average of my choices. But also think we should wait perhaps until erlfmt reaches 1.0 and maybe gather more feedback (possibly with a post on dev@couchdb?). Also wonder if there any chance convincing erlfmt authors to change their mind about a few of the most disliked features here such as `->` being on its own line.
   
   Another issue is being able to cherry-pick commits between `3.x` and `main`. We have two recent PRs, for example, (Erlang 24 support and a pending one for moving chttpd/httpd settings around) that would apply neatly to both branches, but if we reformat one branch those won't apply as easily. Not a deal breaker, but perhaps we would run erlfmt on both main and 3.x at the same time and maybe coordinating with pending PR authors.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org