-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
GH-88597: Added command line interface to UUID module. #99463
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
Conversation
Hi @nanjekyejoannah, as you reviewed the first PR for this issue, would you be able to take a look at this one when you get a chance? I ended up moving the CLI logic to a separate function and importing & using argparse only in there to keep the rest of the module lightweight. |
81c6b06
to
99df7ef
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.
Thank you!
I know that not all CLI entrypoints are tested, but I think we should start doing it. Especially because you have logic in your main
, not just some other function call.
31edacd
to
cc423c8
Compare
a0416f1
to
e7fa43b
Compare
Hi @sobolevn, thanks for taking a look! I believe I've addressed all the comments, and have added in some unit tests. Would you be able to take a look again when you have time? |
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.
Thanks! Several more ideas / comments :)
I think this can also be added to whatsnew/3.12
, because this feature is quite useful.
Hi @sobolevn! I've tried to address all the comments you provided, would you be able to take another look? |
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.
LGTM! But, this will require a core-dev acceptance as well.
thanks! |
For next PRs to Python repos, please don’t force push, it makes reviews less nice (comments can lose context, github notifs don’t link to a useful page, etc). See https://devguide.python.org/getting-started/pull-request-lifecycle/index.html |
UUID module now supports command line usage.