-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
AVRO-3051: Reformatted C++ sources to make them consistent #1154
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.
Hello! I checked that there's no functional changes, just whitespace and formatting, so LGTM from that point. I have no opinion on the formatting used.
It would be great to put the name of the beautifier tool and/or settings in the lang/c++/readme file for future reference as a best practice. I find that very helpful when contributing to a new project!
I added a new config file |
cf195c4
to
dd88360
Compare
Awesome for the script, thanks! LGTM, I checked that:
|
Skimmed and looks good to me, as well. A heads up to the ML might still be worthwhile because people like to bikeshed this stuff. As a follow-up it would be great to add a CI rule that verifies applying PRs are consistent with clang-format. |
Make sure you have checked all steps below.
Jira
Tests
Commits
Documentation