Code review comment for lp:~waigani/juju-core/cmd_help_aliases_1299120

Revision history for this message
William Reade (fwereade) wrote :

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#newcode77
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#oldcode378
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#newcode274
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#newcode375
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#newcode285
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/client.go
File state/apiserver/client/client.go (right):

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

https://codereview.appspot.com/92080046/

« Back to merge proposal