switch: more script friendly output

switch: more script friendly output

Changed switch command to more script friendly
output. Branch has been newly created after an
error when reverting.

https://code.launchpad.net/~themue/juju-core/054-env-more-script-friendly/+merge/191838

(do not edit description out of merge proposal)

[mue, 2013-10-18 16:49:49]

Please take a look.

[gz, 2013-10-18 17:41:49]

Code looks fine apart from one possibly spurious struct member addition.

I have mixed feelings about making switch less user friendly and more script
friendly. It feels mostly like a user-only command to me, unlike status say. In
bzr it's similar, status gives output that's quite machine parsable, but switch
is an interactive command. In the bug, stub doesn't really go into *why* he
wants to script this, my instinct would be to say using seperate juju homes
would be more suitable than having one and switching.

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

https://codereview.appspot.com/15080044/diff/1/cmd/juju/switch.go#newcode22
cmd/juju/switch.go:22: Raw     bool
What's this used for?

[mue, 2013-10-18 18:54:13]

On 2013/10/18 17:41:49, gz wrote:
> Code looks fine apart from one possibly spurious struct member addition.
> 
> I have mixed feelings about making switch less user friendly and more script
> friendly. It feels mostly like a user-only command to me, unlike status say.
In
> bzr it's similar, status gives output that's quite machine parsable, but
switch
> is an interactive command. In the bug, stub doesn't really go into *why* he
> wants to script this, my instinct would be to say using seperate juju homes
> would be more suitable than having one and switching.
> 
> https://codereview.appspot.com/15080044/diff/1/cmd/juju/switch.go
> File cmd/juju/switch.go (right):
> 
> https://codereview.appspot.com/15080044/diff/1/cmd/juju/switch.go#newcode22
> cmd/juju/switch.go:22: Raw     bool
> What's this used for?

Ooops, the raw has indeed to be removed, it's a leftover of a former try.

The discussion about the output has gone through several circles, with --raw and
raw as default. This is the agreement after the discussion.

The problem is that this command is overloaded. It gets, sets and lists for the
user and for scripts.

[rog, 2013-11-12 16:11:26]

LGTM with one thought below.

Personally I think that the script friendly output is also user friendly -
verbosity does not necessarily imply friendliness.

Also many people write ad-hoc scripts on the command line - it's good to be an
upstanding command line citizen and play nicely with the shell IMHO.

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

https://codereview.appspot.com/15080044/diff/1/cmd/juju/switch.go#newcode102
cmd/juju/switch.go:102: fmt.Fprintf(ctx.Stdout, "%s (*)\n", name)
I'm not sure it's worth showing this.
It means that this won't work, for example:

for i in `juju switch -l`; do
    juju status -e $i
done

I'm not sure we need to combine the two
functionalities (list environments and print
current environment name). Let's keep it as
simple as possible.

YMMV though.

[william.reade, 2013-11-18 10:46:13]

This looks like a perfect situation for a --format param and some structs, and
leaving the existing output unchanged as the "smart" (haha) option. What led us
do do it otherwise?

[mue, 2013-11-18 11:04:05]

On 2013/11/18 10:46:13, william.reade wrote:
> This looks like a perfect situation for a --format param and some structs, and
> leaving the existing output unchanged as the "smart" (haha) option. What led
us
> do do it otherwise?

There has been a first approach:
https://code.launchpad.net/~themue/juju-core/053-env-more-script-friendly

The reviews and the discussion for it led to a second approach. Different people
have different preferences. I'll talk to Stuart about his preferences, he "owns"
the issue.

[rog, 2013-11-18 14:05:05]

On 18 November 2013 10:46,  <william.reade@canonical.com> wrote:
> This looks like a perfect situation for a --format param and some
> structs, and leaving the existing output unchanged as the "smart" (haha)
> option. What led us do do it otherwise?

This was a very new command.

I thought that changing it now would result in cleaner
code and a somewhat slicker interface with minimal
compatibility issues. I don't think the verbose
output adds any particular value.

This was all discussed quite a while ago.

[william.reade, 2013-11-18 14:16:40]

On 2013/11/18 14:05:05, rog wrote:
> On 18 November 2013 10:46,  <mailto:william.reade@canonical.com> wrote:
> > This looks like a perfect situation for a --format param and some
> > structs, and leaving the existing output unchanged as the "smart" (haha)
> > option. What led us do do it otherwise?
> 
> This was a very new command.
> 
> I thought that changing it now would result in cleaner
> code and a somewhat slicker interface with minimal
> compatibility issues. I don't think the verbose
> output adds any particular value.
> 
> This was all discussed quite a while ago.

Fair enough, I guess I just missed that one. Thanks for the history. I'm +1 on
your drop-the-(*) suggestion FWIW.

[mue, 2013-11-18 15:42:48]

On 2013/11/18 14:16:40, william.reade wrote:
> On 2013/11/18 14:05:05, rog wrote:
> > On 18 November 2013 10:46,  <mailto:william.reade@canonical.com> wrote:
> > > This looks like a perfect situation for a --format param and some
> > > structs, and leaving the existing output unchanged as the "smart" (haha)
> > > option. What led us do do it otherwise?
> > 
> > This was a very new command.
> > 
> > I thought that changing it now would result in cleaner
> > code and a somewhat slicker interface with minimal
> > compatibility issues. I don't think the verbose
> > output adds any particular value.
> > 
> > This was all discussed quite a while ago.
> 
> Fair enough, I guess I just missed that one. Thanks for the history. I'm +1 on
> your drop-the-(*) suggestion FWIW.

Fine, then I'll do it that way.

[mue, 2013-11-18 16:49:39]

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

https://codereview.appspot.com/15080044/diff/1/cmd/juju/switch.go#newcode22
cmd/juju/switch.go:22: Raw     bool
On 2013/10/18 17:41:49, gz wrote:
> What's this used for?

Ouch, old approach, missed to remove it. Thx.

https://codereview.appspot.com/15080044/diff/1/cmd/juju/switch.go#newcode102
cmd/juju/switch.go:102: fmt.Fprintf(ctx.Stdout, "%s (*)\n", name)
On 2013/11/12 16:11:27, rog wrote:
> I'm not sure it's worth showing this.
> It means that this won't work, for example:
> 
> for i in `juju switch -l`; do
>     juju status -e $i
> done
> 
> I'm not sure we need to combine the two
> functionalities (list environments and print
> current environment name). Let's keep it as
> simple as possible.
> 
> YMMV though.

Done.

[mue, 2013-11-18 17:00:07]

Please take a look.