Fix #11073 - Bump clang-format from 10 to 19 #11081
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I'm doing this ``` clang-format-10 --style=file -i --fallback-style=none -dump-config > .clang-format-10 ``` Then comparing
….clang-format-10 && mv .clang-format-10 .clang-format
…clang-format-10) The correct config value is "Standard: c++11", this is why it was misunderstood as "Latest"...
``` $ clang-format --version Ubuntu clang-format version 18.1.8 (++20240731025043+3b5b5c1ec4a3-1~exp1~20240731145144.92) ```
…p-config --assume-filename=.clang-format > .clang-format-18
This is coming from an ubuntu:noble image from 2 days ago, so it has 18.1.8 as well (edit: it has 18.1.3...) https://github.com/jidicula/clang-format-action/pkgs/container/clang-format/420191370?tag=18
…nts"
```python
def run_clang_tidy_check(fp: Path):
cmd_args = [
'clang-tidy',
"--checks=-*,readability-braces-around-statements",
"--format-style=file",
'--fix', '--quiet',
str(fp)
]
subprocess.run(cmd_args)
```
This reformatted things that weren't fixed via clang-tidy because of preprocessor definitions, or because the file failed to compile (missing declaration Sched:: namespace in Fans.hh for eg)
This is Visual Studio 2022, 17.14.0 (fully up to date) cd "C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\Llvm\bin" .\clang-format.exe --version clang-format version 19.1.5 jidicula action will get plucky's version which is 19.1.7 (noble gets 19.1.1, but if you add the apt.llvm.org deb source you get 19.1.7)
…at differently but not crash) as far back as clang-format-15 (InsertBraces was added there)
… diff differently between clang format 18 and 19
Switches from using the `jidicula/clang-format-action` to a custom, in-house implementation for running clang-format. Benefit: * Only run for changed files for a Pull Request, NOT the entire code base (it takes 4min to run for src/ and tst/ and both will individually spin a new docker container) * Run clang-format in parallel, not serially like the jidicula action does * For a PR: uploads a patch diff that can be applied locally * For a push to develop, this will AUTOCORRECT + do another commit that adds it to .git-blame-ignore-revs * I'm providing clearer logs + a github step summary markdown table
20 tasks
Member
|
And closing this since it's not needed, right @jmarrec ! |
20 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request overview
Fixes Clang Format woefully out of date #11073
I initially bumped to 18, which is what Ubuntu has. But Latest MSVC using clang-format 19.1.5, and this is harder for windows users to install a specific version
Issue was that there was formatting differences (minor) being 18 and 19, but you CANNOT configure that (it's the internal parser), so for these I basically did a bit of refactoring where possible, and I turned off clang format for that (it's like 2 lines in src/ and 3 in tst/)
There are 0 formatting differences in clang-format-18, 19 and 20 right now!
I have redone the clang format action to move away from jidicula to a custom one
I ran clang-tidy-18 with
--checks=-*,readability-braces-around-statements, and I enabled theInsertBraces: truein clang-format as discussed with @Myoldmopar previouslyTHIS SHOULD PROBABLY BE SQUASHED AND ADDED to .git-blame-ignore-revs before merging
Please merge this one instead
@Myoldmopar this is the PR to review, but #11082 should be the one merged (I should have made this one a draft)
New clang format action
Replaces clang-format action with custom implementation
Benefit:
.git-blame-ignore-revsPull Request Author
Reviewer