-
Notifications
You must be signed in to change notification settings - Fork 4k
[ci][c++] fixed whitespace/indent_namespace errors from cpplint
#7056
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
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 always in favor of automation for code layout :)
Fine for me as long as all changes were automatically performed by the linter (I only checked superficially) ✅
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 support this. I'd written "ignore indefinitely" for this check only because I expected it would be a huge diff and didn't personally have a strong preference for either format.
But given that you've already done the work, I'm happy to merge this, to improve the consistency of the code and enforce that automatically going forward. And happy that src/c_api.cpp didn't need to be changed 😅
After this is merged, would you consider doing a followup PR adding this PR's commit to https://github.com/microsoft/LightGBM/blob/master/.git-blame-ignore-revs? That'd skip the mostly-indentation changes in the git blame.
|
Close-reopen to fix |
Refer to #7002.
Initially @jameslamb wrote
But given that fixing this error for current codebase is quite straightforward and touches only 10 files, we can merge it and force this setting for the future PRs for the sake of consistent codebase.