Skip to content

Address all formatting warnings from linters#11977

Closed
hcho3 wants to merge 9 commits intodmlc:masterfrom
hcho3:fix_formatting
Closed

Address all formatting warnings from linters#11977
hcho3 wants to merge 9 commits intodmlc:masterfrom
hcho3:fix_formatting

Conversation

@hcho3
Copy link
Collaborator

@hcho3 hcho3 commented Jan 29, 2026

  • Fix all warnings from pre-commit.
  • Exclude whitespace checks from cpplint. Let clang-format handle whitespace formatting.

I used the AI assistance to resolve many of the cpplint warnings.

Tips for the reviewer: Make sure to hide whitespaces when reviewing. I will split this PR into smaller PRs, once pre-commit run --all-files passes without any warning.

@hcho3 hcho3 changed the title Fix formatting Address all formatting warnings from linters Jan 29, 2026
@trivialfis
Copy link
Member

Oh ...

Comment on lines +30 to +32
exclude: (dmlc-core|gputreeshap|demo\/c-api|jvm-packages\/xgboost4j\/src\/native\/xgboost4j\..*)
additional_dependencies:
- cpplint==1.6.1
- cpplint
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: JNI files and C API demos have been excluded from cpplint checks.

@@ -68,7 +68,15 @@ def __init__(self) -> None:
",".join(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Some checks in cpplint have been turned off.

@hcho3 hcho3 marked this pull request as draft January 29, 2026 19:04
@hcho3
Copy link
Collaborator Author

hcho3 commented Jan 29, 2026

Turning this to draft for now. I didn't realize that #11953 would uncover this many formatting issues ...

@hcho3
Copy link
Collaborator Author

hcho3 commented Jan 29, 2026

TODOs

  • Resolve all compilation errors. Clang-format reordered #include's and uncovered many missing headers in the process. For example, src/common/probability_distribution.h fails to include <xgboost/base.h> for the XGBOOST_DEVICE macro.
  • Run pre-commit run --all-files in the CI.
  • Turn off more checks to make the necessary change size smaller.
  • Split off this PR into smaller PRs, each PR containing at most 20 files.

@trivialfis
Copy link
Member

trivialfis commented Jan 29, 2026

I suggest we fix the headers first using #11790

  • It doesn't override the main code logic. We can handle header changes.
  • It's actually not trivial. I tried. Useful to get it out of the way as the first step.

@RAMitchell
Copy link
Member

RAMitchell commented Jan 30, 2026

Lets not do it this way :) I didn't intend anyone to run pre-commit run --all-files (yet). The changes should happen slowly only on changed files. Lets keep the exclusions you discovered and otherwise try to work one commit at a time.

The main problem I see right now is that we are running one of our python lint scripts in pre-commit which touches every file - it should be split into its component tools in the pre-commit script and run only on changed files.

@hcho3 hcho3 closed this Jan 30, 2026
@hcho3 hcho3 deleted the fix_formatting branch January 30, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants