cmd/juju: confirm destroy-environment

cmd/juju: confirm destroy-environment

Fixes bug 1121914

https://code.launchpad.net/~axwalk/juju-core/lp1121914-confirm-destroy-environment/+merge/177820

(do not edit description out of merge proposal)

[axw, 2013-07-31 12:27:31]

Please take a look.

[fwereade, 2013-07-31 13:12:43]

LGTM, thank you.

[rog, 2013-07-31 13:41:26]

LGTM for backwards compatibility, despite
my meh about the functionality itself.

Please reference bug 1121914 in the description
and attach it to the merge proposal.

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

https://codereview.appspot.com/12158043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:32: f.BoolVar(&c.assumeYes, "yes", false, "Do not
ask for confirmation")
You can delete the flag description from this line - it's taken from
the shortest alias.

https://codereview.appspot.com/12158043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:51: return nil
I think this should probably return an error here.
The destroy-environment command has not succeeded.

https://codereview.appspot.com/12158043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:62: Continue [y/N]? `
s/N/n/ ?

[axw1, 2013-08-01 01:34:31]

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

https://codereview.appspot.com/12158043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:32: f.BoolVar(&c.assumeYes, "yes", false, "Do not
ask for confirmation")
On 2013/07/31 13:41:26, rog wrote:
> You can delete the flag description from this line - it's taken from
> the shortest alias.

Done.

https://codereview.appspot.com/12158043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:51: return nil
On 2013/07/31 13:41:26, rog wrote:
> I think this should probably return an error here.
> The destroy-environment command has not succeeded.

Done.

https://codereview.appspot.com/12158043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:62: Continue [y/N]? `
On 2013/07/31 13:41:26, rog wrote:
> s/N/n/ ?

I don't think so. It's taken straight from pyjuju, and capitalising the default
option is fairly standard IMHO. If there's a preferred way of conveying the
default (e.g. "[y/n] (default: n)?") then I'm happy to do a follow-up.

[axw, 2013-08-01 01:36:03]

Please take a look.

[jameinel, 2013-08-08 18:01:44]

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

https://codereview.appspot.com/12158043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:62: Continue [y/N]? `
On 2013/08/01 01:34:32, axw1 wrote:
> On 2013/07/31 13:41:26, rog wrote:
> > s/N/n/ ?
> 
> I don't think so. It's taken straight from pyjuju, and capitalising the
default
> option is fairly standard IMHO. If there's a preferred way of conveying the
> default (e.g. "[y/n] (default: n)?") then I'm happy to do a follow-up.

I agree. It is standard practice to capitalize the default option.