-
Notifications
You must be signed in to change notification settings - Fork 89
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 --version #182
base: devel
Are you sure you want to change the base?
Add --version #182
Conversation
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.
That's a super great thing you've done, that might help in debugging issues related to changes in the dependencies.
Though I got a few design comments!
def cli(): # pragma: no cover | ||
try: | ||
sys.exit(main(docopt(__doc__.format(self=sys.argv[0].split(os.path.sep)[-1], version=__version__)))) | ||
sys.exit(main(docopt(__doc__.format(self=sys.argv[0].split(os.path.sep)[-1], version=__version__), |
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.
the --help
version should also contain --version
@@ -586,9 +588,39 @@ def main(args): | |||
return 2 | |||
|
|||
|
|||
class Version: |
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.
This should be moved to either:
- the tools module if left as is ;
- the kwargparse module if integrated to the kwargparse/dispatcher system
- its own new module otherwise (i.e.: as a new independant module)
@@ -26,6 +27,9 @@ | |||
import os, json, platform | |||
|
|||
|
|||
SERVICE_PACKAGE = pybitbucket |
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 don't really like that pattern, it feels a bit redundant with the register target decorator. What I'd rather do is either make a new decorator that manages version, or add an argument to register_target
with the version string.
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.
the other better side of that is that you don't need the full loading of the module into the main system, and if a module has a version string that's not standard (i.e. not using __version__
we're good for that.
No description provided.