-
Notifications
You must be signed in to change notification settings - Fork 610
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
Move clang-format outside the sanity check #1012
Move clang-format outside the sanity check #1012
Conversation
This pull request is ready for review |
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.
Nice cleanup! I would suggest also modify CONTRIBUTING.md
so that developers know how to format cpp codes (as make code-format
won't format cpp codes after this change). Thank you.
@WindQAQ done |
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.
Thanks.
Linked to #991
Uses: https://github.com/DoozyX/clang-format-lint-action
The check runs in 20s but we need to upgrade the clang format version. We're using 3.8 but the stable version is 10 now. The github action that we're using supports version 5 to 9.
I propose we bit the bullet now and bump the clang format version to 9.
Instructions to install clang-format-9:
format all with:
The github action didn't support google style out of the box.
See DoozyX/clang-format-lint-action#3
I made a pull request for it: See DoozyX/clang-format-lint-action#5
and currently In the github file, I use a commit on my branch.