Thanks for the datapoint, Wesley! On May 18, 2021, at 10:20 AM, Wesley Bland <w...@wesbland.com<mailto:w...@wesbland.com>> wrote:
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 Pre-commit hook that doubles as a CI checker - 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<mailto: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<mailto:jsquy...@cisco.com> -- Jeff Squyres jsquy...@cisco.com<mailto:jsquy...@cisco.com>
