Skip to content

THRIFT-6219: ci(cpp): Check formatting of changed C++ lines only - #3976

Draft
slachiewicz wants to merge 1 commit into
apache:masterfrom
slachiewicz:THRIFT-6219
Draft

slachiewicz wants to merge 1 commit into
apache:masterfrom
slachiewicz:THRIFT-6219

Conversation

@slachiewicz

Copy link
Copy Markdown
Member

Adds a CI job that runs clang-format 18 on the lines a pull request changes in lib/cpp, so .clang-format is enforced from now on without reformatting existing code.

Six options are added to .clang-format so that it describes lib/cpp as it is written: FixNamespaceComments, ReflowComments, SortIncludes, BinPackArguments, SortUsingDeclarations and SpacesInLineCommentPrefix. The Base64 decode table is excluded with // clang-format off. With both, formatting all of lib/cpp/src changes 104 files and 1,583 lines, down from 149 files and 2,674. The remainder are files with a local style of their own (indented #if, nested namespaces on one line, aligned enum values) that no global option matches.

The job does not cover compiler/cpp or test/cpp.

Verified: git-clang-format-18 --diff HEAD^ -- lib/cpp exits 1 on a misformatted line in a .tcc file, and 0 on a correctly formatted change and on a change without C++ files.

  • Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

Generated-by: Claude Opus 5.5

Client: cpp

The added .clang-format options describe lib/cpp as it is written, so
formatting a change no longer rewrites lines it did not touch. Do not
drop them without reformatting lib/cpp in the same change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@slachiewicz

Copy link
Copy Markdown
Member Author

@Jens-G, is this the direction you want for THRIFT-6219? It takes the ticket's second option, adjusting .clang-format to the code, and checks only changed lines instead of reformatting lib/cpp once. If you would rather do the one-time reformat, the tuned config makes that commit 104 files instead of 149, and the job can then check whole files.

@mergeable mergeable Bot added c++ Pull requests that update C++ code github_actions Pull requests that update GitHub Actions code labels Sep 28, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Pull requests that update C++ code github_actions Pull requests that update GitHub Actions code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant