Skip to content

CI: clang-format-check - run only if changes in c/cpp/header files #5739

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pesekon2
Copy link
Contributor

No description provided.

@pesekon2 pesekon2 self-assigned this May 22, 2025
@echoix
Copy link
Member

echoix commented May 22, 2025

Since it is a required check, we should always have it run.
It is (one of) the fastest of the checks that we have. 30-35 seconds.
clang-format can also format js, json, java and more:
https://clang.llvm.org/docs/ClangFormat.html#standalone-tool

  --assume-filename=<string>     - Set filename used to determine the language and to find
                                   .clang-format file.
                                   Only used when reading from stdin.
                                   If this is not passed, the .clang-format file is searched
                                   relative to the current working directory when reading stdin.
                                   Unrecognized filenames are treated as C++.
                                   supported:
                                     CSharp: .cs
                                     Java: .java
                                     JavaScript: .js .mjs .cjs .ts
                                     Json: .json .ipynb
                                     Objective-C: .m .mm
                                     Proto: .proto .protodevel
                                     TableGen: .td
                                     TextProto: .txtpb .textpb .pb.txt .textproto .asciipb
                                     Verilog: .sv .svh .v .vh

@pesekon2
Copy link
Contributor Author

Since it is a required check, we should always have it run.

Are you sure? What is the purpose of running it if no change in the PR could cause any change in its result?

It is (one of) the fastest of the checks that we have. 30-35 seconds.

This is true but in our case (many many many checks), it is good to get rid of anything that is not needed. Also, we can burn fewer trees by saving some runs. I plan to move to some of the more demanding ones now but I wanted to start with the quick fixes.

clang-format can also format js, json, java and more: https://clang.llvm.org/docs/ClangFormat.html#standalone-tool

Hmm, any idea then why Java is not mentioned among the supported extensions in the docs of the tool we use: https://github.com/DoozyX/clang-format-lint-action?tab=readme-ov-file#extensions?

@github-actions github-actions bot added the CI Continuous integration label May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants