-
Notifications
You must be signed in to change notification settings - Fork 260
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
Add file formatting checks to CI #209
Conversation
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 for this! I have a few comments, but I do like the general idea and I have used it before the upcoming release despite not merging it yet.
8b4aec6
to
cbb0907
Compare
cbb0907
to
ff82d6f
Compare
@rpavlik Requesting review and merge. I'm not sure what else I can do with this PR, I've responded to your feedback as well as I can. I think the best course of action is to merge it, and then you can make any further changes as needed. |
sorry for not merging this sooner - it looks good, I was just having some trouble getting it to run on our CI. I did run it before the last release, I think, despite not being able to merge. This one is on me now. |
Thanks - figured out my wsl problem and got it merged! |
Follow-up to #208. The formatting script runs on every text file except for those in
external/
andsrc/external/
.Fingers crossed that the CI checks will work, in previous implementations I had this script set up through GitHub Actions, but since this repo uses Azure Pipelines I wrote a config for that by copying the clang-format one. It doesn't seem that the new checks are running with this PR, that might change if it's merged but I'm not sure, I'm not familiar with Azure Pipelines.