Code review comment for lp:~fwereade/juju-core/api-force-destroy-machines

just some of my thoughts, not a full review.

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

https://codereview.appspot.com/24790044/diff/1/state/api/client.go#newcode154
state/api/client.go:154: func (c *Client) ForceDestroyMachines(machines
...string) error {
On 2013/11/12 00:43:42, wallyworld wrote:
> It makes me sad that we need to add a whole new endpoint because Go
doesn't
> support default parameter values :-(

It isn't really an endpoint, but it is a new function.

I only make the distinction because I was about to say the next change:
-func (c *Client) DestroyServiceUnits(unitNames []string) error {
+func (c *Client) DestroyServiceUnits(unitNames ...string) error {

was an API break, but this is all just within the client process.

so we *could* spell it:

DestroyMachines(force bool, machines ...string)

I'm guessing it was a bit ackward and William wanted to avoid bool in
APIs especially for a Force sort of function.

https://codereview.appspot.com/24790044/diff/1/state/apiserver/client/client_test.go
File state/apiserver/client/client_test.go (right):

https://codereview.appspot.com/24790044/diff/1/state/apiserver/client/client_test.go#newcode549
state/apiserver/client/client_test.go:549: assertLife(c, m0,
state.Alive)
On 2013/11/12 00:43:42, wallyworld wrote:
> Why doesn't force destroy cause the life to be set to Dying? I find it
> particularly unintuitive that a status would show the machine as alive
when in
> fact it is on death row and waiting for the clean up job to remove it.

In earlier discussions, this is a limitation of the Machine lifecycle
model. A Machine can't be set to dying until all Units are no longer on
the machine.

https://codereview.appspot.com/24790044/