-
Notifications
You must be signed in to change notification settings - Fork 908
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
Remove includes suggested by include-what-you-use #17170
Remove includes suggested by include-what-you-use #17170
Conversation
@vyasr Can we confirm if this has any effect on clean build times? |
Since wheel builds are not currently sccaching at all, a quick test would be to see how the wheel-build-libcudf runtime compares to another recent PR (say #17168). |
Unfortunately I think those build times are too noisy to derive much information from. A clean local build may be the only way to collect that information reliably. |
That's fine. I am happy to accept this as an improvement without hard evidence of decreasing build times. I don't think it is worth the engineering time to answer that definitively. |
I also tried this locally on a full build (cleared ccache) and did not see any changes in the build time. |
Cool thanks for checking. Whether or not we move forward with #17078, I think we might as well merge this PR as a simple clean up. |
@davidwendt could you please approve this if you are happy with it based on our discussion from today's meeting? |
/merge |
Description
This PR cherry-picks out the suggestions from IWYU generated in #17078.
Checklist