-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add cargo udeps as a new CI check #9792
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?
Conversation
| # name of this job must be unique across all workflows | ||
| # otherwise GitHub will mark all these jobs as required | ||
|
|
||
| cargo-udeps: |
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.
I'm not convinced to add this, there seems to be a lot of false positives that you need to handle.
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.
I see your point. I think the false positives would be a real problem if we had no way to deal with them. But since we can safely ignore them, I believe it’s still a useful check
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.
But it clutters the Cargo.toml files.
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.
IMO the trade-off is worth it — just a few extra lines to prevent unused dependencies, which would otherwise lead to bigger binaries and longer compilation times
franciscoaguirre
left a comment
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.
For me, the trade-off of having to ignore false positives in exchange for less useless dependencies is worth it
|
Updated based on the latest changes and unused deps |
Description
This PR removes unused dependencies, marks some others to be ignored, and introduces a
cargo-udepscheck in CI to prevent regressions.It finalizes my work started in #9235, ensuring all unnecessary dependencies are cleaned up across the project.
Note: Since there are well known false positives I've checked each one and ignore the ones that we are 100% are false positives
Closes: #6906