-
Notifications
You must be signed in to change notification settings - Fork 870
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
Add --group
support to uv add
and uv remove
#8108
Conversation
a94726f
to
9d0e1e0
Compare
This comment was marked as outdated.
This comment was marked as outdated.
47fdf22
to
fc36784
Compare
1c476cd
to
89efa56
Compare
@@ -190,6 +217,49 @@ impl From<Metadata> for RequiresDist { | |||
} | |||
} | |||
|
|||
fn apply_source_strategy( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was somewhat surprised that this is only used once. I guess the other "dependency types" (like production) don't match this exact pattern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I actually moved it out because I thought I would need to apply it twice, like.. to the dev-dependencies
group but I ended up structuring things differently. I think we can move it back in, or make it an impl method somewhere.
}) | ||
.collect::<Result<Vec<_>, _>>()?; | ||
|
||
dependency_groups.into_iter().collect::<BTreeMap<_, _>>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this correctly merge keys? Like if dev is included twice, does this concatenate the values? I sort of think it might replace them...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it probably replaces them, yeah. I wasn't a goal to resolve that here (per the description) since this implemented before I wrote a proposal. I figured I'd write some test cases and address it separately for review purposes.
Part of #8090 Adds the ability to add and remove dependencies from arbitrary groups using `uv add` and `uv remove`. Does not include resolving with the new dependencies — tackling that in #8110. Additionally, this does not yet resolve interactions with the existing `dev` group — we'll tackle that separately as well. I probably won't merge the stack until that design is resolved.
Part of #8090 Adds the ability to add and remove dependencies from arbitrary groups using `uv add` and `uv remove`. Does not include resolving with the new dependencies — tackling that in #8110. Additionally, this does not yet resolve interactions with the existing `dev` group — we'll tackle that separately as well. I probably won't merge the stack until that design is resolved.
Part of #8090 Adds the ability to add and remove dependencies from arbitrary groups using `uv add` and `uv remove`. Does not include resolving with the new dependencies — tackling that in #8110. Additionally, this does not yet resolve interactions with the existing `dev` group — we'll tackle that separately as well. I probably won't merge the stack until that design is resolved.
Part of #8090 Adds the ability to add and remove dependencies from arbitrary groups using `uv add` and `uv remove`. Does not include resolving with the new dependencies — tackling that in #8110. Additionally, this does not yet resolve interactions with the existing `dev` group — we'll tackle that separately as well. I probably won't merge the stack until that design is resolved.
Part of #8090 Adds the ability to add and remove dependencies from arbitrary groups using `uv add` and `uv remove`. Does not include resolving with the new dependencies — tackling that in #8110. Additionally, this does not yet resolve interactions with the existing `dev` group — we'll tackle that separately as well. I probably won't merge the stack until that design is resolved.
Part of #8090 Adds the ability to add and remove dependencies from arbitrary groups using `uv add` and `uv remove`. Does not include resolving with the new dependencies — tackling that in #8110. Additionally, this does not yet resolve interactions with the existing `dev` group — we'll tackle that separately as well. I probably won't merge the stack until that design is resolved.
Part of #8090 Adds the ability to add and remove dependencies from arbitrary groups using `uv add` and `uv remove`. Does not include resolving with the new dependencies — tackling that in #8110. Additionally, this does not yet resolve interactions with the existing `dev` group — we'll tackle that separately as well. I probably won't merge the stack until that design is resolved.
Part of #8090
Adds the ability to add and remove dependencies from arbitrary groups using
uv add
anduv remove
. Does not include resolving with the new dependencies — tackling that in #8110.Additionally, this does not yet resolve interactions with the existing
dev
group — we'll tackle that separately as well. I probably won't merge the stack until that design is resolved.