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

Add IWYU pragmas #1008

Merged
merged 5 commits into from
Mar 20, 2024
Merged

Add IWYU pragmas #1008

merged 5 commits into from
Mar 20, 2024

Conversation

gostefan
Copy link
Contributor

Added include-what-you-use pragmas to:

  • let IWYU point users to the main include file CLI/CLI.hpp
  • tell IWYU that CLI/CLI.hpp is the main exporting header.

This should fix #816

Copy link

codecov bot commented Feb 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (e4ee3af) to head (3e7e3e2).
Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main     #1008    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           17        17            
  Lines         4546      4557    +11     
  Branches         0       971   +971     
==========================================
+ Hits          4546      4557    +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@phlptp
Copy link
Collaborator

phlptp commented Feb 20, 2024

This looks OK to me. I am going to give @henryiii a few days to comment, then will merge.

Copy link
Collaborator

@henryiii henryiii left a comment

Choose a reason for hiding this comment

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

Not very familiar with IWYU pragmas, but seems fine.

@henryiii
Copy link
Collaborator

henryiii commented Feb 26, 2024

Oh, it is valid though to include individual headers (it's in the readme). It sounds like this will just point users to always including the conglomerate header? Shouldn't these (Config and formatter) have // IWYU pragma: keep instead?

@gostefan
Copy link
Contributor Author

I looked through the Readme and didn't find any references to individually including headers except for the utilities (which I promptly forgot to correctly handle in my changes).

Is it ok to include the "regular" headers (e.g. Argv.hpp) individually?
I'll adapt the PR accordingly.

@phlptp phlptp merged commit 2fa609a into CLIUtils:main Mar 20, 2024
54 checks passed
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.

clangd warnings on transitive includes
3 participants