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

Removed dependencies that cargo-machete claimed were unnecessary #21995

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sureshjoshi
Copy link
Member

This is a naive slash. I haven't looked into each dependency yet to see if "does it compile" is enough of a metric to safely remove the dep.

Also, I need to review the lock file to see if any of the constraints version changes matter

@sureshjoshi
Copy link
Member Author

This was a fail. cargo build is not representative of the dependencies we need/use in our project - as it looks like cargo test requires deps that machete misses.

@tgolsson
Copy link
Contributor

tgolsson commented Feb 25, 2025

I guess those should be in [dev-dependencies] then, so might still save some time in recompilation.

@sureshjoshi
Copy link
Member Author

I guess those should be in [dev-dependencies] then, so might still save some time in recompilation.

Yep, I'm working on a different ticket right now - but I was going to go through and re-do what I was doing with cargo test as well as build/check.

We should always have dependencies as localized as possible, so yeah, those should definitely be dev-deps, not regular deps.

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.

2 participants