Silently allow --no-update for backward compatibility #10164
Conversation
Reviewer's Guide by SourceryThis pull request introduces support for the '--no-update' flag to allow backward compatibility by silently allowing its usage. The changes ensure that when '--no-update' is provided with the lock command, the locked packages are not updated, and proper error handling has been added when both '--no-update' and '--regenerate' are used simultaneously. Test cases have been updated to verify the new behavior. Updated class diagram for LockCommand in LockCommand handlingclassDiagram
class LockCommand {
+List options
+handle() int
}
%% New option added for backward compatibility
class LockCommandOptions {
+"--no-update": boolean
+"--regenerate": boolean
}
LockCommand --> LockCommandOptions : uses
note for LockCommand "The '--no-update' option is added for backward compatibility. In handle(), if both '--no-update' and '--regenerate' are present, an error is displayed and returns 1."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leonardgerardatomicmachinescom - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using
pytest.mark.parametrizeto avoid repeating the same assertion logic intest_no_update_and_regenerate_at_the_same_time.
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.
leonardgerardatomicmachinescom
force-pushed
the
main
branch
2 times, most recently
from
881fa35 to
4a71826
Compare
leonardgerardatomicmachinescom
force-pushed
the
main
branch
from
4a71826 to
1ddc346
Compare
|
Not sure if this still makes sense after such a long time and so few complaints (resp. so few "thumbs up" on this PR). I think we should have printed a deprecation warning instead of just removing it in the first place but now it is probably just fine to leave it alone? |
|
Consequence for us is that we stuck to poetry v1 and will probably move to some other toolchain rather than upgrade to v2. Note that some other cli breakage led to the full decision, a major one being v2 |
Does that mean that this PR would still help you if included in the next minor release or that it does not matter to you? In case, you are still interested in getting this merged, would it be fine for you if we print a deprecation warning if
I think that one was fixed in #9831 with Poetry 2.0.0. |
2 participants
The error
The option "--no-update" does not existis not necessary since no update is anyway the default. Erroring does not help. Contrarily, it prevents scripts or automated uses to work with both v1 and v2.(I work in a place with shared build scripts, but users control which version of poetry they use, so when one installs poetry v2, it fails with the --no-update flag error).
Pull Request Check List
Resolves: #issue-number-here
Summary by Sourcery
Tests:
--no-updateflag and its incompatibility with the--regenerateflag.