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

Make all dependencies workspace dependencies #9333

Merged
merged 1 commit into from
Jan 2, 2024
Merged

Conversation

charliermarsh
Copy link
Member

Summary

This PR modifies our Cargo.toml files to use workspace dependencies for all dependencies, rather than the status quo of sporadically trying to use workspace dependencies for those dependencies that are used across multiple crates. I find the current situation more confusing and harder to manage, since we have a mix of workspace and crate-local dependencies, whereas this setup consistently uses the same approach for all dependencies.

@charliermarsh charliermarsh added the internal An internal refactor or improvement label Dec 31, 2023
Copy link
Contributor

github-actions bot commented Dec 31, 2023

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

Base automatically changed from charlie/flake8-to-ruff to main December 31, 2023 16:09
@konstin
Copy link
Member

konstin commented Jan 2, 2024

Big 👍 keeping all the [dependencies] tables consistent. One disadvantage is that this pulls [dev-dependencies] and ruff_dev into [workspace.dependencies], so the top level dependency table contains crates that the ruff binary doesn't require, e.g. tempfile and imara-diff (that's not our diff library, we use similar, only ruff_dev uses imara-diff for perf reasons).

@charliermarsh
Copy link
Member Author

👍 Makes sense -- maybe I'll move ruff_dev to use non-workspace dependencies for everything, actually.

@charliermarsh
Copy link
Member Author

Hmm, I started to do this (move all dev dependencies out of the workspace) but then kinda changed my mind. It still seems good to have them use consistent declarations.

@charliermarsh charliermarsh enabled auto-merge (squash) January 2, 2024 13:40
@charliermarsh charliermarsh merged commit 9073220 into main Jan 2, 2024
16 checks passed
@charliermarsh charliermarsh deleted the charlie/workspace branch January 2, 2024 13:42
Copy link
Member

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this. I was also unsure of when to add something as a workspace dep and when not to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants