-
Notifications
You must be signed in to change notification settings - Fork 18
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
Adding version flag #31
Adding version flag #31
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.
Please fix the Python 2.x compatibility of your change and then I can merge it.
repositorytools/cli/common.py
Outdated
self.parser.add_argument("-D", "--debug", action="store_true", dest="debug", default=False, | ||
help="Print lots of debugging information") | ||
self.parser.add_argument("-D", "--debug", action="store_true", help="Print lots of debugging information") | ||
self.parser.add_argument("-V", action="store_true", dest="display_version", help="Prints version and exit") |
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.
please add also long form --version
if you want is as an option.
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.
Not possible sadly, you already used that flag:
https://github.com/stardust85/repositorytools/blob/master/repositorytools/cli/commands/artifact.py#L30
repositorytools/cli/common.py
Outdated
self.repository = None | ||
|
||
def run(self, args=None): | ||
args_namespace = self.parser.parse_args(args) | ||
if args_namespace.display_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 functionality didn't work for me in Python 2.6 or 2.7 for some reason. It exited, but didn't print anything.
Please try a solution working also in python >= 2.6
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.
It works for me with Python 2.7, but you have to feed the subparser:
$ artifact -V get-metadata X Y
repositorytools v4.2.2
This is due to a bug / change in argparse
between Python 2 & 3: https://stackoverflow.com/a/24211480/636849
I haven't found any easy workaround so far. The error is raised here: https://github.com/python/cpython/blob/2.7/Lib/argparse.py#L1949
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.
If you don't like this behaviour, feel free to ignore this PR, and potentially include this feature by yourself later on.
This commit was only planned as a minor improvement, I don't think it's worth a convoluted workaround :)
repositorytools/cli/common.py
Outdated
@@ -55,5 +58,5 @@ def run(self, args=None): | |||
return args_namespace.func(args_namespace) | |||
|
|||
def __call__(self, *args): | |||
self.run(args) | |||
self.run(*args) |
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.
Merged in #30, but you can probably keep it here, because github doesn't complain about it ;)
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.
Yes, that was expected. I left this commit so that this adding_version_flag
branch could be merged once you included the *args
fix.
I rebased this PR. Is this ok for you ? |
Hi, please merge my changes from today to your branch. They fix broken python2.6 and python 3.3 tests. Btw, would you mind if I gave you r/w access to repositorytools? :) |
Done |
OK, I sent you an invitation to packagemanagement organization. |
No description provided.