It really is a shame that could not go forward. There are really three end 
goals in mind:

1) Consistency. We all have different coding styles and following a common 
coding style is more and more considered a best practice. The number of 
projects using clang-format grows continuously. I find it mildly annoying when 
someone changes the coding style in code I maintain because I end up having to 
fix it for consistency.

2) clang-tidy. This is the ultimate end goal. clang-tidy can find real problems 
in the code and help to reduce debugging time down the road. It has a huge 
community behind it and is constantly improving. Think of it like lint on speed.

3) Correctness. clang-format doesn't do much of this on its own but sorting the 
include lines found real errors in header dependencies. The fixing of 
indentation can also help to expose real coding issues during development that 
may be hidden by poor indentation (this has been a problem in Open MPI in the 
past). Other tools can do this part but we loose clang-tidy.

All in all the formatting was not really that bad beyond a few corner cases. 
Usually when I see these I rearrange the code to make it look better. One 
example (which Open MPI should recommend) is the trailing comma in 
initializers. It makes clang-format output much cleaner.

Anyway, I have said my peace and will continue to clang-format my code whenever 
I modify it :).

-Nathan

On May 17, 2021 at 2:01 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