cmd/juju: destroy-service command

cmd/juju: destroy-service command

...plus some very minor consistency fixes in the same package.

https://code.launchpad.net/~fwereade/juju-core/cli-destroy-service/+merge/143800

(do not edit description out of merge proposal)

[fwereade, 2013-01-18 00:26:02]

Please take a look.

[dimitern, 2013-01-22 13:51:57]

LGTM, nice!

[rog, 2013-01-25 16:35:58]

LGTM

[rog, 2013-01-25 16:36:07]

LGTM

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

https://codereview.appspot.com/7138062/diff/1/cmd/juju/destroyservice.go#newc...
cmd/juju/destroyservice.go:32: } else if !state.IsServiceName(args[0]) {
s/else//
?
(if there's an else, i always have to look twice to see if it's referring to a
variable declared locally in the previous if)

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

https://codereview.appspot.com/7138062/diff/1/cmd/juju/destroyservice_test.go...
cmd/juju/destroyservice_test.go:26: // Create two services with a relation
between them.
i think this is overkill, in all honest, as Service.Destroy is well tested in
state. if we just created a service, destroyed it and checked that it was gone,
that would be sufficient. 

that said, if you think this is testing specific jujud functionality, by all
means leave it in.

[fwereade, 2013-01-25 17:10:26]

*** Submitted:

cmd/juju: destroy-service command

...plus some very minor consistency fixes in the same package.

R=dimitern, rog
CC=
https://codereview.appspot.com/7138062

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

https://codereview.appspot.com/7138062/diff/1/cmd/juju/destroyservice.go#newc...
cmd/juju/destroyservice.go:32: } else if !state.IsServiceName(args[0]) {
On 2013/01/25 16:36:08, rog wrote:
> s/else//
> ?
> (if there's an else, i always have to look twice to see if it's referring to a
> variable declared locally in the previous if)

Fair enough -- I rather like chunking tests that I consider related into the
same if/else construct, but it doesn't seem to help other people's understanding
the way it does mine, so I should probably try to lose the habit ;).

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

https://codereview.appspot.com/7138062/diff/1/cmd/juju/destroyservice_test.go...
cmd/juju/destroyservice_test.go:26: // Create two services with a relation
between them.
On 2013/01/25 16:36:08, rog wrote:
> i think this is overkill, in all honest, as Service.Destroy is well tested in
> state. if we just created a service, destroyed it and checked that it was
gone,
> that would be sufficient. 
> 
> that said, if you think this is testing specific jujud functionality, by all
> means leave it in.

It's not, really. Done.