pnoltes opened a new pull request, #834:
URL: https://github.com/apache/celix/pull/834

   This PR introduces a **clang-tidy** setup for the project.
   
   I know we previously introduced the GCC static analyzer, but was already 
testing **clang-tidy** in the background. A key benefit of clang-tidy is its 
compatibility with both Clang and GCC. Additionally, you do not need to perform 
a full compilation first, though clang-tidy does require the CMake-generated 
`compile_commands.json` file.
   
   I have enabled most check groups but disabled individual checks that 
currently report issues. I expect we can enable almost all checks over time (if 
desired), though some will require further discussion.
   
   ### Potentially Problematic Checks:
   
   * **cppcoreguidelines**: Almost all of these checks trigger on C headers 
included in C++. This might be solvable by wrapping `extern "C"` directives 
with `//NOLINTBEGIN(cppcoreguidelines-*)` and 
`//NOLINTEND(cppcoreguidelines-*)`.
   * **readability-***: Some of these are highly opinionated. For example, 
`readability-identifier-naming` flags certain variable names as too short 
(e.g., `bnd`).
   
   ### CI/CD Integration
   
   To run clang-tidy, I added a separate workflow. While it is possible to 
append a clang-tidy scan to an existing build job, doing so would extend the 
total execution time. Since clang-tidy only requires a CMake configuration, 
running it as a parallel job avoids increasing the overall build duration.
   
   For local development, the `ENABLE_CLANG_TIDY` build option can be used to 
integrate the scan into the build process. While this is also possible on CI, I 
believe a single dedicated scan is sufficient.
   
   The scan uses the `clang-tidy-sarif` tool to convert the output into a 
**SARIF report**. GitHub supports this format, though I am still confirming 
exactly how these results integrate into our specific GitHub environment (e.g., 
the Security tab).
   
   ### Final Notes:
   
   * I have temporarily disabled the suppression of the `cert-dcl59-cpp` check 
so that there is are least some findings to report for testing purposes.
   * The `cargo-bins/cargo-binstall` action is restricted in our organization, 
so `clang-tidy-sarif` is currently "installed" via `curl`. I have kept the 
action usage in comments for now. If we agree that clang-tidy is the right path 
forward and the `curl` approach is acceptable, I will remove the commented-out 
code.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to