Skip to content

Make all dependencies workspace dependencies#9333

Merged
charliermarsh merged 1 commit intomainfrom
charlie/workspace
Jan 2, 2024
Merged

Make all dependencies workspace dependencies#9333
charliermarsh merged 1 commit intomainfrom
charlie/workspace

Conversation

@charliermarsh
Copy link
Copy Markdown
Member

@charliermarsh charliermarsh commented Dec 31, 2023

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
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 31, 2023 •
edited
Loading

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
Copy Markdown
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
Copy Markdown
Member Author

charliermarsh commented Jan 2, 2024

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

@charliermarsh
Copy link
Copy Markdown
Member Author

charliermarsh commented Jan 2, 2024

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
@charliermarsh charliermarsh deleted the charlie/workspace branch January 2, 2024 13:42
BurntSushi
BurntSushi reviewed Jan 4, 2024
View reviewed changes
Copy link
Copy Markdown
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

Reviewers

@BurntSushi BurntSushi BurntSushi left review comments

@konstin konstin konstin approved these changes

@MichaReiser MichaReiser Awaiting requested review from MichaReiser MichaReiser is a code owner

Assignees

No one assigned

Labels

internal An internal refactor or improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@charliermarsh @konstin @BurntSushi