LightGBM icon indicating copy to clipboard operation
LightGBM copied to clipboard

[style] Introduce clang-format

Open borchero opened this issue 8 months ago • 1 comments

Motivation

Every time I touch the C++ code, I have to manually format it. This always feels annoying and it's impossible to ensure style consistency.

To make it simpler for myself and other contributors to write easily readable C++ code, this PR introduces clang-format which is (to the best of my knowledge) the de-facto standard for formatting in the C/C++ ecosystem.

Changes

  • Add a .clang-format file with the basic style configuration based on the "Google" style (which cpplint also follows)
  • Add clang-format to the .pre-commit-config.yaml
  • Adjust cpplint to not generate false positives after the formatting
  • Ignore most parts of include/LightGBM/config.h and all of src/io/config_auto.cpp from formatting (the latter via .clang-format-ignore) as clang-format messes up the current parameter documentation otherwise
  • Remove duplicate semicolons in three places which made cpplint fail after formatting

Once this PR is merged, we should probably add the merge commit to .git-blame-ignore-revs.

In a follow-up PR, we might want to replace cpplint with clang-tidy as a much more powerful alternative.

borchero avatar Apr 27 '25 12:04 borchero

but a +16,379 −17,112 PR is way too large to review.

Fair enough. However, I think breaking up the PR in the same way as for the introduction of yamllint is neither viable not necessary. As clang-format does not yield linting failures that need to be fixed manually but auto-formats the code, I don't think its changes require careful review. As long as the CI passes, I am pretty certain that there can't be an issue. According to this Stackoverflow answer, the only way that clang-format can break the code is by sorting imports (which I'd propose to simply disable for the sake of the introducing PR).

borchero avatar Apr 27 '25 19:04 borchero