-
-
Notifications
You must be signed in to change notification settings - Fork 428
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: pass dev selection to sync on pdm add and remove #3415
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3415 +/- ##
=======================================
Coverage 85.23% 85.23%
=======================================
Files 112 112
Lines 11486 11486
Branches 2516 2516
=======================================
Hits 9790 9790
Misses 1171 1171
Partials 525 525
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
LGTM, pdm add seems also suffer from the same issue, can you change it too? |
@@ -174,7 +174,7 @@ def do_add( | |||
if sync: | |||
do_sync( | |||
project, | |||
selection=GroupSelection(project, groups=[group], default=False), | |||
selection=GroupSelection(project, groups=[group], dev=selection.dev, default=False), |
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.
Oops, that fails a test case, when the group is a dev group but --dev
is not passed explicitly. I guess it's not the correct way to fix, neither for pdm remove
.
I think you should fix the _translated_groups()
function, so that when dev groups don't exist in the lock file they won't be added to the group_set
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.
Gotcha, I will take a look later.
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.
You can use a set intersection to exclude the non-exist groups
Deprecated due to #3420 |
Pull Request Checklist
news/
describing what is new.Describe what you have changed in this PR.
A trial fix to #3404.
Actually, I am not sure whether this is the desired approach to fix. Any comments are welcome.