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

chore: Remove default_members from Cargo.toml #3006

Merged
merged 1 commit into from
Feb 19, 2023

Conversation

MichaReiser
Copy link
Member

This PR removes the default_members from the workspace configuration.

Why

I'm not familiar with the motivation for why the default_members setting was added initially, and I do not object to keeping it. I'll explain my motivation for removing it below.

My main reason for removing the default_members override is that new contributors may not know that cargo test, cargo build, and other commands only run on a subset of crates. They may then be surprised that their PRs are failing in CI, but everything works locally.

My guess why default_members was added is to speed up the development workflow. That's fair, but I question the value because ruff is the heaviest crate to build.

@charliermarsh charliermarsh merged commit a7c5336 into astral-sh:main Feb 19, 2023
@charliermarsh
Copy link
Member

(Agreed.)

@charliermarsh
Copy link
Member

Oh, I guess the reason this was present was to make ruff_cli the default run invocation. You now have to do cargo run -p ruff_cli to run Ruff from root.

@charliermarsh
Copy link
Member

Not sure if there's a way to fix that, or if we need to update the docs anywhere.

JonathanPlasse added a commit to JonathanPlasse/ruff that referenced this pull request Feb 19, 2023
Add --force-exclude, --exit-non-zero-on-fix to ruff pre-commit hook
JonathanPlasse added a commit to JonathanPlasse/ruff that referenced this pull request Feb 19, 2023
Add --force-exclude, --exit-non-zero-on-fix to ruff pre-commit hook
charliermarsh pushed a commit that referenced this pull request Feb 19, 2023
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.

None yet

2 participants