hsf-training / cpluspluscourse

C++ Course Taught at CERN, from Sebastien Ponce (LHCb)
Apache License 2.0
157 stars 58 forks source link

Add a clang-format style file. #477

Closed hageboeck closed 1 year ago

hageboeck commented 1 year ago

Here is a draft of a clang-format style based on WebKit.

I propose to agree on a style and merge the .clang-format file without changing every single file immediately. People editing with clang-format-capable editors will automatically check in formatted code when they work on a file. After a while, when we are happy with the style it imposes, we can write a loop over all files and create a commit.

Four files have already been formatted using the draft style, see:

sponce commented 1 year ago

In my experience (in both acceptations of the terms actually), this is more tricky than it seems, as the indentation will vary with versions of clang... Thus if you have locally a different version than I do, we'll fight about indentation. The consequence is that the indentation can only be enforced in the gitlab CI, which means that it should fail when indentation is not fine and give a URL where to download the diff that people will apply by hand locally before they force push... Really heavy, but the way we work in LHCb

Another possibility is of course to share a common version of clang, but then we have to take it e.g. from cvmfs which means that you lose the ability to work offline.

As I said, no so easy... Any strong feeling in which direction we should go ?

bernhardmgruber commented 1 year ago

the indentation can only be enforced in the gitlab CI, which means that it should fail when indentation is not fine and give a URL where to download the diff that people will apply by hand locally before they force push...

Wow, this is exactly how LLAMA CI works :D :D I also implemented it the same way for alpaka and a few other projects. What does not work great is to push the clang-format changes to the branch from the CI. That fails sometimes.

Another possibility is of course to share a common version of clang, but then we have to take it e.g. from cvmfs which means that you lose the ability to work offline.

Hmm. I get my clang from LLVM's apt, so I have the newest version a day after it was released :) I have seen active projects at CERN still on clang-format 3.x something, because they agreed on a common version. So maybe let's not go there :)

As I said, no so easy... Any strong feeling in which direction we should go ?

For now I would do like @hageboeck said. Don't enforce it, but have a style file for new code or when we modify exercises. We can then fix the style at some point in the future and add a CI run. It's not too hard. And clang-format is mostly stable across versions now. I see like 1 of a few 1000 lines change whenever I upgrade. I think we can tolerate that.

sponce commented 1 year ago

And clang-format is mostly stable across versions now. I see like 1 of a few 1000 lines change whenever I upgrade. I think we can tolerate that.

You indeed have a point here. I see the same, which is clearly a drama with a project containing > 10M lines of code. but of course we are not in that case, so here it should not be too bad indeed. Let's try.