-
-
Notifications
You must be signed in to change notification settings - Fork 948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clang-format: Update to 16 #2139
base: main
Are you sure you want to change the base?
Conversation
Build size and comparison to main:
|
c34df9a
to
88e5c64
Compare
Sorry for the force push spam 😅. GitHub doesn't have an easy way to test CI locally, so I just did it in this PR. The failing clang-tidy test is because the infinitime-build container still uses Ubuntu 22.04. I think the reason we use the docker container in that check is to generate the |
Yeah may as well do it all in one go I think, it makes sense to do the upgrade atomically |
I've actually just thought that we don't necessarily need to update clang-tidy at the same time as clang-format, they don't interact at all. I might just revert the clang-tidy version bump and keep this PR to only updating clang-format. |
88e5c64
to
715a279
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. It makes sense to keep the clang tooling at the same version to me though - tools get installed at the same time so the versions will be the same on real systems. In what scenario would you have clang-format from clang 16 installed but clang-tidy from 14 - it seems like we'd be using a very strange configuration here
@@ -17,7 +17,7 @@ do | |||
src/libs/*|src/FreeRTOS/*) continue ;; | |||
*.cpp|*.h) | |||
echo "::group::$file" | |||
clang-tidy-14 -p build "$file" || true | |||
clang-tidy-16 -p build "$file" || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If clang-tidy isn't being updated, this should be undone right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, oops
I agree that clang-tidy 14 with clang-format 16 is a bit of a strange setup, but I think that it's fine for the interim while I get clang-tidy 16 working. |
Oh, what's up with clang-tidy 16? |
As discussed in #2127.
I've also updated the clang-format options to be compatible, and sorted the options.