Fix #11073 - Bump clang-format from 10 to 19 #11082
Merged
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
Main change is bumping to clang-format-19, enabling InsertBraces: true. For the CI action, replace jidicula with custom action 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 f4d009
20 tasks
Member
|
Alright this will be great. It will be so much easier for our team since it will be an updated version of Clang Format. Visual Studio should do a better job of auto-formatting. No more having to track down old versions of Clang Format. And yes, it does run so much faster now, thanks @jmarrec! I understand this will cause some conflicts, but they should be very straightforward. Spacing/line breaks, basic formatting changes. I anticipate being to remedy the conflicts on each branch as I'm reviewing today. If I get stuck, I'll just checkout the branch version of the file and then apply updated Clang format locally. Off I go. Thanks @jmarrec ! |
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
This is a squash of Fix #11073 - Bump clang-format from 10 to 19 #11081
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 previouslyDescription of the purpose of this PR
Pull Request Author
Reviewer