Fix 1299120 command aliases

Fix 1299120 command aliases

Fix bug 1299120 by ensuring that the
preferred command is not displayed as
an alias of  another command in juju 
help commands.

Change file and function names to
be consistant with the preferred 
command.

https://code.launchpad.net/~waigani/juju-core/cmd_help_aliases_1299120/+merge/218728

(do not edit description out of merge proposal)

[waigani, 2014-05-08 02:50:40]

Please take a look.

[waigani, 2014-05-08 05:03:21]

Please take a look.

[fwereade, 2014-05-08 06:43:46]

This breaks compatibility in some places, and we can't do that. The changes in
cmd/juju are fine, but anything that changes what we send over the wire for the
API is not.

https://codereview.appspot.com/92080046/diff/20001/cmd/juju/removemachine.go
File cmd/juju/removemachine.go (right):

https://codereview.appspot.com/92080046/diff/20001/cmd/juju/removemachine.go#...
cmd/juju/removemachine.go:77: return
apiclient.ForceReplaceMachines(c.MachineIds...)
Replace?

https://codereview.appspot.com/92080046/diff/20001/state/api/client.go
File state/api/client.go (left):

https://codereview.appspot.com/92080046/diff/20001/state/api/client.go#oldcod...
state/api/client.go:378: // ServiceDestroy destroys a given service.
What about service? Did we keep "destroy" for service but "remove" everything
else? that seems somewhat incoherent to me.

https://codereview.appspot.com/92080046/diff/20001/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/92080046/diff/20001/state/api/client.go#newcod...
state/api/client.go:274: func (c *Client) ReplaceMachines(machines ...string)
error {
This Replace is surprising to me. Just a typo, or something deeper I haven't
figured out?

https://codereview.appspot.com/92080046/diff/20001/state/api/client.go#newcod...
state/api/client.go:375: return c.call("RemoveServiceUnits", params, nil)
We definitely can't change the stuff we call() -- we need to keep working with
old servers.

https://codereview.appspot.com/92080046/diff/20001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/92080046/diff/20001/state/api/params/params.go...
state/api/params/params.go:285: type RemoveServiceUnits struct {
Yeah, please just don't touch the API at all. We can use the better names with
new api versions -- when we get versioning -- but for now this is just a break.

(ok, *this* is not -- the names of the types aren't reflected in what we sent
over the wire)

https://codereview.appspot.com/92080046/diff/20001/state/apiserver/client/cli...
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/92080046/diff/20001/state/apiserver/client/cli...
state/apiserver/client/client.go:490: func (c *Client) RemoveServiceUnits(args
params.RemoveServiceUnits) error {
...but *this* is a break.

[waigani, 2014-05-08 21:29:35]

Please take a look.

[fwereade, 2014-05-09 06:51:55]

LGTM, but please do the type/var renames for RemoveMachine as well

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/main.go
File cmd/juju/main.go (left):

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/main.go#oldcode83
cmd/juju/main.go:83: jujucmd.Register(wrap(&DestroyMachineCommand{}))
Shouldn't we RemoveMachine as well? if not, why not? :)

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/removemachine.go
File cmd/juju/removemachine.go (left):

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/removemachine.go#...
cmd/juju/removemachine.go:24: const destroyMachineDoc = `
removeMachineDoc

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/removemachine.go#...
cmd/juju/removemachine.go:31: func (c *DestroyMachineCommand) Info() *cmd.Info {
RemoveMachineCommand

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/removemachine.go
File cmd/juju/removemachine.go (right):

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/removemachine.go#...
cmd/juju/removemachine.go:34: # Remove machine 6, running units or containers
Remove machine 6, and all its units and containers?

[jameinel, 2014-05-09 09:24:49]

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/main.go
File cmd/juju/main.go (left):

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/main.go#oldcode83
cmd/juju/main.go:83: jujucmd.Register(wrap(&DestroyMachineCommand{}))
On 2014/05/09 06:51:55, fwereade wrote:
> Shouldn't we RemoveMachine as well? if not, why not? :)

It was intended to switch to "juju remove-machine" at least in the bug.

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/removemachine.go
File cmd/juju/removemachine.go (left):

https://codereview.appspot.com/92080046/diff/40001/cmd/juju/removemachine.go#...
cmd/juju/removemachine.go:39: }
It seems you can't comment on a rename-only change. I wanted to note that
removemachine_test.go certainly seems like it would need updating for the
changes of DestroyMachine to RemoveMachine.

[waigani, 2014-05-09 22:55:50]

On 2014/05/09 09:24:49, jameinel wrote:
> https://codereview.appspot.com/92080046/diff/40001/cmd/juju/main.go
> File cmd/juju/main.go (left):
> 
> https://codereview.appspot.com/92080046/diff/40001/cmd/juju/main.go#oldcode83
> cmd/juju/main.go:83: jujucmd.Register(wrap(&DestroyMachineCommand{}))
> On 2014/05/09 06:51:55, fwereade wrote:
> > Shouldn't we RemoveMachine as well? if not, why not? :)
> 
> It was intended to switch to "juju remove-machine" at least in the bug.
> 
> https://codereview.appspot.com/92080046/diff/40001/cmd/juju/removemachine.go
> File cmd/juju/removemachine.go (left):
> 
>
https://codereview.appspot.com/92080046/diff/40001/cmd/juju/removemachine.go#...
> cmd/juju/removemachine.go:39: }
> It seems you can't comment on a rename-only change. I wanted to note that
> removemachine_test.go certainly seems like it would need updating for the
> changes of DestroyMachine to RemoveMachine.

Yes, you're right. It seems I'm being sloppy with my commits. I'd made the
changes but managed not to commit them, sorry.

[waigani, 2014-05-09 22:59:21]

Please take a look.