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

Issue 94710046: cmd/juju: rework "juju user add" output

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 10 months ago by menn0
Modified:
9 years, 10 months ago
Reviewers:
waigani, mp+220746
Visibility:
Public.

Description

cmd/juju: rework "juju user add" output As per identity user stories: - JSON output removed, not useful (implementation no longer uses cmd.Output) - YAML .jenv output only to file via --output option, not to screen - Human friendly output now emitted to stdout - ".jenv" is automatically added to output file path if required https://code.launchpad.net/~menno.smits/juju-core/user_add-output/+merge/220746 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+134 lines, -99 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/user_add.go View 5 chunks +61 lines, -26 lines 2 comments Download
M cmd/juju/user_add_test.go View 3 chunks +71 lines, -73 lines 2 comments Download

Messages

Total messages: 4
menn0
Please take a look.
9 years, 10 months ago (2014-05-23 05:47:49 UTC) #1
waigani
LGTM with I learnt a few new packages/functions. Thanks! https://codereview.appspot.com/94710046/diff/1/cmd/juju/user_add.go File cmd/juju/user_add.go (right): https://codereview.appspot.com/94710046/diff/1/cmd/juju/user_add.go#newcode25 cmd/juju/user_add.go:25: ...
9 years, 10 months ago (2014-05-25 22:22:02 UTC) #2
waigani
LGTM with I learnt a few new packages/functions. Thanks! https://codereview.appspot.com/94710046/diff/1/cmd/juju/user_add.go File cmd/juju/user_add.go (right): https://codereview.appspot.com/94710046/diff/1/cmd/juju/user_add.go#newcode25 cmd/juju/user_add.go:25: ...
9 years, 10 months ago (2014-05-25 22:22:02 UTC) #3
menn0
9 years, 10 months ago (2014-05-25 22:56:26 UTC) #4
https://codereview.appspot.com/94710046/diff/1/cmd/juju/user_add.go
File cmd/juju/user_add.go (right):

https://codereview.appspot.com/94710046/diff/1/cmd/juju/user_add.go#newcode25
cmd/juju/user_add.go:25: using -o / --output.
On 2014/05/25 22:22:01, waigani wrote:
> As command generates a file, I first read the / in "-o /" as specifying the
file
> path.

I'll make this clearer.

https://codereview.appspot.com/94710046/diff/1/cmd/juju/user_add_test.go
File cmd/juju/user_add_test.go (right):

https://codereview.appspot.com/94710046/diff/1/cmd/juju/user_add_test.go#newc...
cmd/juju/user_add_test.go:86: }()
On 2014/05/25 22:22:02, waigani wrote:
> Create a suite and move tempFile creation and removal to SetUpTest and
> TestTearDown? Actually, that's probably overkill for one test.

Probably but this is pretty ugly isn't it? I've got some ideas on how to do this
better.
Sign in to reply to this message.

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