You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After edits (see rest of thread):
This repo validates that clang-format v13 has been run on the code, and if the formatting doesn't match will mark a commit as being in error. This should be documented in CONTRIBUTING.md.
We may also want to up this requirement to the latest clang-format version, since 13 is a few years old now, and the formatting that clang-format expects changes over time. The latest version is currently 18.
Before edits: I tried using Tools/format_source.sh on Arch. I had clang-format version 17.0.6 installed
I tried using clang-format from the scoop install llvm package on Windows. Version 18.1.1
Neither of them seem to be matching the clang-format settings that our continuous integration expects.
We should more clearly document whatever our clang-format CI is using. It should be part of CONTRIBUTING.md
I saw a file <root>/.clang-format, but I ran clang-format with that file on the source tree and ended up with zero corrections, even though the latest main build failed the analysis. So the analyzer doesn't seem to be using that config.
Aha. It was because the version specified in the module is clang-format version 13, and apparently clang-format reserves the right to constantly tweak their formatting or something.
Wondering if we should update to v18 since that's the latest?
Still needs to be documented. I will convert this ticket to a documentation ticket, for CONTRIBUTING.md.
I may also want to check in some binaries for windows and a batch file for running it on the whole repo. It's kind of more of a pain on Windows to get the right version of things, since Windows package management has never really taken off. For example, I had to file bugs against scoop install llvm@13.0.1 since that was broken. Maybe chocolatey is better. But none of them are nearly as well trodden as linux/mac package managers.
shinymerlyn
changed the title
Clang format verification may be using some weird combo of clang format settings
clang-format 13 requirement needs documentation
Mar 18, 2024
After edits (see rest of thread):
This repo validates that clang-format v13 has been run on the code, and if the formatting doesn't match will mark a commit as being in error. This should be documented in CONTRIBUTING.md.
We may also want to up this requirement to the latest clang-format version, since 13 is a few years old now, and the formatting that clang-format expects changes over time. The latest version is currently 18.
Before edits:
I tried usingTools/format_source.sh
on Arch. I hadclang-format
version 17.0.6 installedI tried using clang-format from the
scoop install llvm
package on Windows. Version 18.1.1Neither of them seem to be matching the clang-format settings that our continuous integration expects.We should more clearly document whatever our clang-format CI is using. It should be part of CONTRIBUTING.md@feliwir @redruin1 - I think you may have set it up?
The text was updated successfully, but these errors were encountered: