Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(873)

Issue 6593044: Add --version option

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 6 months ago by clint.byrum
Modified:
8 years, 7 months ago
Reviewers:
jimbaker, mp+127109
Visibility:
Public.

Description

Add --version option https://code.launchpad.net/~clint-fewbar/juju/add-version-arg/+merge/127109 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -0 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M juju/control/__init__.py View 2 chunks +4 lines, -0 lines 1 comment Download
M juju/control/tests/test_control.py View 2 chunks +13 lines, -0 lines 0 comments Download

Messages

Total messages: 2
clint.byrum
Please take a look.
11 years, 6 months ago (2012-09-29 16:33:47 UTC) #1
jimbaker
11 years, 6 months ago (2012-10-03 22:07:57 UTC) #2
LGTM with the minor. One thing I noticed is that I would expect the output on
stdout, not stderr, so that I can do this

export JUJU_VERSION=$(juju --version)

instead of 

export JUJU_VERSION=$(juju --version 2>1)

Of course this behavior of using stderr is just what argparse does. However, in
a completely random sampling of commands, most print to stdout (ls, xargs, bzr).
One notable exception: python, it uses stderr. Same with java -version.

However, supporting --version like this is going to have to work around the fact
that we use argparse to support subcommands. And anything dealing with
subcommands in argparse is possible, but way too complicated.

So, never mind, let's just do what argparse makes easy :)

https://codereview.appspot.com/6593044/diff/1/juju/control/__init__.py
File juju/control/__init__.py (right):

https://codereview.appspot.com/6593044/diff/1/juju/control/__init__.py#newcod...
juju/control/__init__.py:110: "--version", action="version", version='juju %s' %
(__version__))
version="juju %s" % __version__

would be preferable (we use double quotes uniformly in the Juju codebase).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b