Skip to content
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

[CI] speed up clang-tidy workflow #3148

Merged
merged 4 commits into from
Nov 15, 2024

Conversation

sjinks
Copy link
Contributor

@sjinks sjinks commented Nov 15, 2024

Changes

This PR speeds up the clang-tidy workflow. Because clang-tidy only needs a compilation database generated by CMake, there's no need to waste time compiling the source code.

The only target we need to build is opentelemetry_proto because some other files depend on the files generated by the Protobuf compiler.

We can achieve further performance improvements by running clang-tidy jobs in parallel.

Before: 43:06 (number of warnings: 114)
After: 14:30 (number of warnings: 114)

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@sjinks sjinks requested a review from a team as a code owner November 15, 2024 14:11
Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name Link
🔨 Latest commit 3ca6218
🔍 Latest deploy log https://app.netlify.com/sites/opentelemetry-cpp-api-docs/deploys/673756770c06a0000835f5fc

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.82%. Comparing base (497eaf4) to head (3ca6218).
Report is 165 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3148      +/-   ##
==========================================
+ Coverage   87.12%   87.82%   +0.70%     
==========================================
  Files         200      195       -5     
  Lines        6109     6146      +37     
==========================================
+ Hits         5322     5397      +75     
+ Misses        787      749      -38     

see 113 files with indirect coverage changes

@marcalff marcalff changed the title ci: speed up clang-tidy workflow [CI] speed up clang-tidy workflow Nov 15, 2024
Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the CI speedup improvement.

@marcalff
Copy link
Member

@sjinks Thanks for the fix, it is most welcome.

We are falling behind with basic checks like include-what-you-use (almost done) and clang-tidy (preliminary CI was set in place, but cleanup has barely started yet).

Contributions like yours, from someone having both the skills and the willingness to help, are definitively appreciated.

@marcalff marcalff merged commit a388e87 into open-telemetry:main Nov 15, 2024
56 checks passed
malkia added a commit to malkia/opentelemetry-cpp that referenced this pull request Nov 15, 2024
@sjinks sjinks deleted the speedup-clang-tidy branch November 19, 2024 21:45
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.

2 participants