-
Notifications
You must be signed in to change notification settings - Fork 438
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
Add Include what you use #2214
Add Include what you use #2214
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2214 +/- ##
=======================================
Coverage 87.51% 87.51%
=======================================
Files 169 169
Lines 4889 4889
=======================================
Hits 4278 4278
Misses 611 611 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as is.
This allows to see IWYU reports in the build logs, so we can start to take action and do some cleanup, even when reading logs to find issues is tedious (download the build log, search it).
Possible improvements, the CI most likely will need:
- a way to filter out IWYU complaints on code we do not own (third party headers, generated protobuf code)
- a way to summarize which file is / is not compliant, so that reading or searching logs is easier
This can be done later, once the code is cleaned up, so we can enforce a pass/fail result in CI.
Not sure if/how the github CI can report a warning, so it gets visibility in the overall build, and yet allow the build to pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
related to #2054 (issue)
Changes
Adds Include what you use to the CI jobs that use clang.
The CI will not fail, only warning messages will be added e.g. here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes