Skip to content

Conversation

@charliermarsh
Copy link
Member

Summary

Now that default-groups can include more than just "dev", it makes sense to allow users to remove groups with --no-group.

@charliermarsh charliermarsh added the enhancement New feature or improvement to existing functionality label Oct 22, 2024
@charliermarsh charliermarsh requested a review from zanieb October 22, 2024 19:33
/// Exclude dependencies from the specified local dependency group.
///
/// May be provided multiple times.
#[arg(long, conflicts_with("only_group"))]
Copy link
Member

@zanieb zanieb Oct 22, 2024

Choose a reason for hiding this comment

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

I don't think this conflicts? What if I do --only-group foo --only-group bar --no-group foo? Per previous discussions about overriding prior options, this seems nice to permit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I went back and forth on this.

Copy link
Member

Choose a reason for hiding this comment

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

I trust your judgement, I was just surprised.

Copy link
Member Author

Choose a reason for hiding this comment

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

It just added a bit of complexity to the implementation, so I had punted it. Makes sense though.

"#,
)?;

uv_snapshot!(context.filters(), context.sync().arg("--no-group").arg("foo"), @r###"
Copy link
Member

@zanieb zanieb Oct 22, 2024

Choose a reason for hiding this comment

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

What about a test for --group foo --no-group foo?

(not necessarily in this function)

@charliermarsh charliermarsh merged commit 37df9ba into tracking/735 Oct 22, 2024
21 of 22 checks passed
@charliermarsh charliermarsh deleted the charlie/no-group branch October 22, 2024 20:18
@zanieb zanieb mentioned this pull request Oct 22, 2024
charliermarsh added a commit that referenced this pull request Oct 23, 2024
## Summary

Now that `default-groups` can include more than just `"dev"`, it makes
sense to allow users to remove groups with `--no-group`.
charliermarsh added a commit that referenced this pull request Oct 25, 2024
## Summary

Now that `default-groups` can include more than just `"dev"`, it makes
sense to allow users to remove groups with `--no-group`.
charliermarsh added a commit that referenced this pull request Oct 25, 2024
## Summary

Now that `default-groups` can include more than just `"dev"`, it makes
sense to allow users to remove groups with `--no-group`.
charliermarsh added a commit that referenced this pull request Oct 25, 2024
## Summary

Now that `default-groups` can include more than just `"dev"`, it makes
sense to allow users to remove groups with `--no-group`.
zanieb pushed a commit that referenced this pull request Oct 25, 2024
## Summary

Now that `default-groups` can include more than just `"dev"`, it makes
sense to allow users to remove groups with `--no-group`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or improvement to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants