-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
fix: update
, add
and remove
shall not uninstall extra dependencies
#10016
fix: update
, add
and remove
shall not uninstall extra dependencies
#10016
Conversation
Reviewer's Guide by SourceryThis pull request changes the behavior of Poetry with regards to handling extra dependencies during package installation. Previously, installing a package with extras would remove any other installed extras not explicitly specified. With this change, Sequence diagram for updated package installation behavior with extrassequenceDiagram
participant User
participant Poetry
participant PackageManager
User->>Poetry: poetry install -E A -E B
Note over Poetry: Package has extras A, B, C
Note over Poetry: C is already installed
Poetry->>PackageManager: Calculate operations
PackageManager-->>Poetry: Operations list
Note over Poetry: New behavior: Keep C
Poetry->>PackageManager: Install/Update packages
PackageManager-->>Poetry: Installation complete
Poetry-->>User: Success (A, B installed, C kept)
User->>Poetry: poetry sync
Note over Poetry: Sync removes unspecified extras
Poetry->>PackageManager: Calculate operations
PackageManager-->>Poetry: Operations list (includes C removal)
Poetry->>PackageManager: Remove unspecified extras
PackageManager-->>Poetry: Sync complete
Poetry-->>User: Success (only A, B remain)
State diagram for extras handling behaviorstateDiagram-v2
[*] --> HasExtras: Initial state with
extras A, B, C installed
HasExtras --> PartialExtras: poetry install -E A -E B
HasExtras --> SyncedExtras: poetry sync --extras "A B"
PartialExtras --> SyncedExtras: poetry sync --extras "A B"
state PartialExtras {
[*] --> Installed: A, B installed
Installed --> Kept: C kept but not requested
}
state SyncedExtras {
[*] --> Required: A, B installed
Required --> Removed: C removed
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
a128a78
to
507ca98
Compare
update
, add
and remove
with shall not uninstall extra dependenciesupdate
, add
and remove
shall not uninstall extra dependencies
507ca98
to
861dd4a
Compare
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.
Hey @radoering - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please update the documentation to reflect this behavior change regarding how extra dependencies are handled during install/update/sync operations.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Good point. |
861dd4a
to
0b3eea0
Compare
Deploy preview for website ready! ✅ Preview Built with commit 7bc1baf. |
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.
Hey @radoering - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
0b3eea0
to
4fd6971
Compare
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.
Hey @radoering - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
With this change unrequested extras dependencies will also be kept when running `install` and are only removed when running `sync`!
4fd6971
to
7bc1baf
Compare
With this change unrequested extras dependencies will also be kept when running
install
and are only removed when runningsync
!Pull Request Check List
Resolves: #9975
Requires: #10014
Summary by Sourcery
Bug Fixes: