Update formatting and linting #2691
Conversation
|
I get errors when the lint script calls |
So |
|
Can you please verify (just print it with |
Yes.
|
|
Nevermind, running the format script with the all flag made several changes and the lint script is happy now. Do you want me to push these changes? |
No, please don't. Because most changes are simply from a style change after clang 18. Since we currently validate with clang 18, your changes would fail in the CI. |
|
Why does this PR not update the install_dependencies.sh and the jenkinsfile? I believe that the pipeline is still using clang-tidy 17. |
|
I think I have discovered part of the problem. If you turn on clang-tidy in cmake, then only the cpp files are linted, but not the hpp files. I have opened #2697 , which contains instructions for clang-tidy to also lint included header files. In the pipeline you can see that the clang-tidy job produces a lot of errors in our header files. I am not sure if this is everything, but maybe it makes sense to include this in here? It would blow up the PR tho, so we can also do this in a separate PR. There is also some way to tell cmake/clang-tidy to autofix their issues. I can also take a look into that, but that seemed to have some other problems: https://discourse.cmake.org/t/whats-the-idiomatic-way-to-run-clang-tidy-on-header-files-with-cmake/9530 I am also not sure if this finds all the issues that are being highlighted by clangd in vscode, but it definitely finds quite a few of them (and some that are not even highlighted by clangd) |
Not sure what you mean. This PR does not touch the CI pipeline. Moving to a more recent clang version is another branch (which is stale for a while unfortunately) and is not super simple as we need to address many things for clang tidy. |
Saw that PR. Nice catch. Thanks! |
I believe you were talking about clang-20 here. Maybe this is because homebrew already uses clang-20 and our pipeline is still stuck at clang-17. Maybe it makes sense to upgrade the pipeline, so we don't run into any issues there? |
Updating is definitely a good idea. But I need to find the time for that. There is already a branch that has 80-90% of the fixes, but I will not find the time to finish this PR the next couple of weeks. |
Following the preparations for DYOD, this PR adapts the lint script to use clang-format checking.
To pass clang-format, this PR also includes running
clang-format all. Apparently, we have not done that for a while. Most changes are simply a different styling that clang-format now applies.