Hey folks,

As a datapoint, in MPICH, we use GNU indent 
<https://www.gnu.org/software/indent/> for this.

Script to format code (either one file or the entire code tree) - 
https://github.com/pmodels/mpich/blob/main/maint/code-cleanup.bash 
<https://github.com/pmodels/mpich/blob/main/maint/code-cleanup.bash>
Pre-commit hook that doubles as a CI checker - 
https://github.com/pmodels/mpich/blob/3e6c3ae11df33d6c02a025aaa5b145bd262d1c32/maint/hooks/pre-commit#L52-L94
 
<https://github.com/pmodels/mpich/blob/3e6c3ae11df33d6c02a025aaa5b145bd262d1c32/maint/hooks/pre-commit#L52-L94>
 

There are (as always) a few gotchas, but overall it works pretty well. The main 
annoyance we discovered was that at least between version 2.2.11 and 2.2.12, 
there were changes made that changed the output formatting with the same set of 
input arguments, which is obviously a problem if people use different versions. 
So we lock in version 2.2.11. Unfortunately, that version is not the easiest to 
find and by default didn’t work on Mac OS IIRC (patch to fix Homebrew formula 
here <https://gist.github.com/wesbland/501063f151c1eb815d8001abf2285cbe>). You 
might be better served by moving on to the most recent version, but you 
probably want to pick a version and stick with it until you verify that it 
won’t cause problems otherwise. Maybe we should figure out a way to have 
multiple versions live alongside each other so people that work on both 
projects could have both versions. The project doesn’t seem to be particularly 
active so this probably isn’t a problem that will come up often.

We picked K&R formatting as a starting point and then customized a few things 
<https://github.com/pmodels/mpich/blob/3e6c3ae11df33d6c02a025aaa5b145bd262d1c32/maint/code-cleanup.bash#L26-L72>
 based on our tastes. There are relatively sane defaults and a few different 
“sets” of defaults depending on what you prefer.

Let me know if you have any questions. I’d be happy to provide any pointers.

Thanks,
Wes

> On May 17, 2021, at 2:59 PM, Jeff Squyres (jsquyres) via devel 
> <devel@lists.open-mpi.org> wrote:
> 
> FYI: It was decided last week that we will abandon the current effort to 
> reformat master / v5.0.x according to style rules.
> 
> SHORT VERSION
> 
> We have already reformatted opal/ and tests/.  But the two PRs for 
> reformatting ompi/ brought up a whole bunch of issues that do not seem 
> resolvable via clang-format.  As such, we're just walking away: we're not 
> going to revert the reformatting that was done to opal/ and tests/ on master 
> and v5.0.x, but we're just going to close the ompi/ reformatting PRs without 
> merging.
> 
> Many thanks to Nathan who invested a lot of time in this; I'm sorry it didn't 
> fully work out.  :-(
> 
> MORE DETAIL
> 
> It turns out that clang-format actually parses the C code into internal 
> language primitives and then re-renders the code according to all the style 
> choices that you configure.  Meaning: you have to make decisions about every 
> single style choice (e.g., whether to put "&&" at the beginning or end of the 
> line, when expressions span multiple lines).
> 
> This is absolutely not what we want to do.  
> https://github.com/open-mpi/ompi/wiki/CodingStyle is intentionally very 
> "light touch": it only specifies a bare minimum of style rules -- the small 
> number of things that we could all agree on.  Everything outside of those 
> rules is not regulated.
> 
> Clang-format simply doesn't work that way: you have to make a decision for 
> every single style choice.
> 
> So we'll close https://github.com/open-mpi/ompi/pull/8816 and 
> https://github.com/open-mpi/ompi/pull/8923 without merging them.
> 
> If someone would like to find a better tool that can:
> 
> a) re-format the ompi/ and oshmem/ trees according to our "light touch" rules
> b) fail a CI test when a PR introduces a delta that results in code breaking 
> the "light touch" rules
> 
> Then great: let's have that conversation.  But clang-format is not going to 
> work for us, unfortunately.  :-(
> 
> -- 
> Jeff Squyres
> jsquy...@cisco.com
> 
> 
> 

Reply via email to