Skip to content

Add IWYU pragmas#1008

Merged
phlptp merged 5 commits into
CLIUtils:mainfrom
gostefan:IWYU_pragmas
Mar 20, 2024
Merged

Add IWYU pragmas#1008
phlptp merged 5 commits into
CLIUtils:mainfrom
gostefan:IWYU_pragmas

Conversation

@gostefan
Copy link
Copy Markdown
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

@codecov
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
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