-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Clean-up libfetchers headers #14261
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
base: master
Are you sure you want to change the base?
Clean-up libfetchers headers #14261
Conversation
[ | ||
{ "include": ["@<nlohmann/.*>", "private", "<nlohmann/json.hpp>", "public"] }, | ||
|
||
# "<git2/XXX.h> => <git2.h> where XXX doesn't contain '/' | ||
{ "include": ["@<git2/[^/]+\\.h>", "private", "<git2.h>", "public"] }, | ||
|
||
] No newline at end of file |
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.
This file now can handle the common mappings we want here as it grows.
@@ -1,4 +1,12 @@ | |||
#pragma once |
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.
This seemed like a small bug missing the pragma
I bet this could be an easy lint?
97de67e
to
d8fa3ef
Compare
#include <git2/repository.h> | ||
#include <nlohmann/json_fwd.hpp> | ||
#include <git2/oid.h> | ||
#include <git2/types.h> |
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.
Interesting that sometimes my mapping file just converts these to git2.h but othertimes it replaces it with more.
This is in the same vein as #14251
Motivation
I ran include-what-you-use over libflake as it seemed small to start.
steps:
I then went through the header files and now I can more safely remove the header files clangd warns that are "not used".
Context
There is an ongoing debate on whether to do include-what-you-use (iwyu).
For:
Against:
Note: You may still see "unused" header declaration warnings by clangd. From what I can understand, since I am running
iwyu
only on partial codebase, it is putting exactly the necessary includes for what is needed however includes from other parts of the codebase may still be giving the declarations for some headers that are included.I saw this notably with includes for
<mutex>
for instance.Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.