-
Notifications
You must be signed in to change notification settings - Fork 41
Add a format checking job and clang-format config #93
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
Conversation
d89a077 to
b5dce22
Compare
b5dce22 to
5b2685b
Compare
jameysharp
left a comment
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.
I'm sad that this changes so much code, but checking and canonicalizing formatting is so valuable that it's worth doing anyway. I spent a little while trying to see if there were settings that might reduce the amount of code churn here, but the source tree wasn't really internally consistent so nevermind.
I do think it's important to not touch wizer.h though since that comes from another project. xqd.h might be in the same situation, but I haven't yet found where the original might be if there is one.
ci/clang-format.ignore
Outdated
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.
c-dependencies/js-compute-runtime/wizer.h is copied from https://github.com/bytecodealliance/wizer/blob/main/include/wizer.h so I think it should also be excluded here.
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.
I've excluded it and xqd.h, thanks for catching this!
ci/clang-format.sh
Outdated
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.
Can I suggest if ! cmp -s "$file" "$formatted"; then instead? It should be faster than diff since you don't care what the differences are, only if there are any. Of note, the -s flag to cmp is standardized in POSIX, but the --quiet and --silent synonyms are GNU extensions.
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.
So much better, thank you!
790532d to
e80259d
Compare
e80259d to
d8941ff
Compare
Add a
.clang-formatconfig in the project root with mostly default settings (line length is set to 100 instead of 80). Also add a ci job to runci/clang-format.shon all tracked c/c++ source.