cmd/juju/destroyenvironment: allow -e flag

cmd/juju/destroyenvironment: allow -e flag

We still require the user to name the environment (we don't allow it
to come from the default environment name). However, this gives a
syntax that will work in both 1.16 and 1.18
(juju destroy-environment -e ENV).

https://code.launchpad.net/~jameinel/juju-core/destroy-environment-e-1248329/+merge/202838

(do not edit description out of merge proposal)

[jameinel, 2014-01-23 11:55:35]

Please take a look.

[rog, 2014-01-23 18:39:07]

A couple of thoughts:

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

https://codereview.appspot.com/56020043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:46: 
We should have "environment" too, as it's a standard alias for -e.
(not that anyone'll actually use it, but still...)

https://codereview.appspot.com/56020043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:99: 
Since 1.16 didn't have environment as a positional argument,
can't we just lose the positional logic entirely?

It seems a bit like unnecessary clutter to me.

[fwereade, 2014-01-24 15:00:09]

LGTM

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

https://codereview.appspot.com/56020043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:46: 
On 2014/01/23 18:39:08, rog wrote:
> We should have "environment" too, as it's a standard alias for -e.
> (not that anyone'll actually use it, but still...)

+1

https://codereview.appspot.com/56020043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:99: 
On 2014/01/23 18:39:08, rog wrote:
> Since 1.16 didn't have environment as a positional argument,
> can't we just lose the positional logic entirely?
> 
> It seems a bit like unnecessary clutter to me.

AIUI, people have had problems accidentally calling destroy-environment -y with
the wrong current environment. Forcing people to type out the name of the
environment they're destroying actually seems like a good response -- there's no
reasonable way to claim juju surprised you with what environment it targeted.

[rog, 2014-01-24 15:59:57]

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

https://codereview.appspot.com/56020043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:99: 
On 2014/01/24 15:00:09, fwereade wrote:
> On 2014/01/23 18:39:08, rog wrote:
> > Since 1.16 didn't have environment as a positional argument,
> > can't we just lose the positional logic entirely?
> > 
> > It seems a bit like unnecessary clutter to me.
> 
> AIUI, people have had problems accidentally calling destroy-environment -y
with
> the wrong current environment. Forcing people to type out the name of the
> environment they're destroying actually seems like a good response -- there's
no
> reasonable way to claim juju surprised you with what environment it targeted.

I'm not saying that people shouldn't have to type out the name of the
environment, just that I don't see there's a particular reason to support both:

    juju destroy-environment -e foo

*and*

    juju destroy-environment foo

Other than the clutter, it also constrains future compatibility-maintaining
changes for the command - we might actually *want* to add a positional argument
in the future.

[fwereade, 2014-01-24 19:10:21]

On 2014/01/24 15:59:57, rog wrote:
> Other than the clutter, it also constrains future compatibility-maintaining
> changes for the command - we might actually *want* to add a positional
argument
> in the future.

We *do* want a positional argument. The -e version is the bit to maintain
compatibility.

[fwereade, 2014-01-24 19:10:27]

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

https://codereview.appspot.com/56020043/diff/1/cmd/juju/destroyenvironment.go...
cmd/juju/destroyenvironment.go:91: 
hey, should we not do a deprecation warning here? I thought the only reason we
kept this around was to keep scripts working in the near run.

[jameinel, 2014-01-28 11:27:56]

Please take a look.

[fwereade, 2014-01-28 11:31:09]

LGTM, thanks