Add API support for ServiceGet.

Add API support for ServiceGet.



https://code.launchpad.net/~bac/juju-core/add-get-2/+merge/150400

(do not edit description out of merge proposal)

[bac, 2013-02-25 18:34:47]

Please take a look.

[rog, 2013-02-26 10:03:35]

LGTM with one minor thought below.

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

https://codereview.appspot.com/7384058/diff/1/state/api/api_test.go#newcode183
state/api/api_test.go:183: service, err := st.Client().ServiceGet("wordpress")
i think i'd prefer to see a separate test for the service get functionality
specifically. the op* functions are an adjunct to TestOperationPerm designed
specifically to test whether given operations are allowed (they do also check
the return values, but that's really only because it's convenient and cheap to
do so.

that's why we have TestClientStatus as well as opClientStatus.

https://codereview.appspot.com/7384058/diff/1/state/api/api_test.go#newcode195
state/api/api_test.go:195: "value":       interface{}(nil),
just nil is fine here.

[bac, 2013-02-26 11:14:40]

Please take a look.

[rog, 2013-02-26 11:17:52]

LGTM still, with one ultra-trivial.

https://codereview.appspot.com/7384058/diff/5001/state/api/api_test.go
File state/api/api_test.go (right):

https://codereview.appspot.com/7384058/diff/5001/state/api/api_test.go#newcod...
state/api/api_test.go:215: "description": "A descriptive title used for the
blog."}}}
trivial, but for consistency, please put the closing braces on their own lines,
as in scenarioStatus above.

[bac, 2013-02-26 12:56:39]

Please take a look.

[bac, 2013-02-26 12:59:34]

Thanks for the reviews Roger.  Everything you requested is done.

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

https://codereview.appspot.com/7384058/diff/1/state/api/api_test.go#newcode183
state/api/api_test.go:183: service, err := st.Client().ServiceGet("wordpress")
On 2013/02/26 10:03:35, rog wrote:
> i think i'd prefer to see a separate test for the service get functionality
> specifically. the op* functions are an adjunct to TestOperationPerm designed
> specifically to test whether given operations are allowed (they do also check
> the return values, but that's really only because it's convenient and cheap to
> do so.
> 
> that's why we have TestClientStatus as well as opClientStatus.

Done.

https://codereview.appspot.com/7384058/diff/1/state/api/api_test.go#newcode195
state/api/api_test.go:195: "value":       interface{}(nil),
On 2013/02/26 10:03:35, rog wrote:
> just nil is fine here.

Done.

https://codereview.appspot.com/7384058/diff/5001/state/api/api_test.go
File state/api/api_test.go (right):

https://codereview.appspot.com/7384058/diff/5001/state/api/api_test.go#newcod...
state/api/api_test.go:215: "description": "A descriptive title used for the
blog."}}}
On 2013/02/26 11:17:52, rog wrote:
> trivial, but for consistency, please put the closing braces on their own
lines,
> as in scenarioStatus above.

Done.

[TheMue, 2013-02-26 13:18:14]

LGTM

[bac, 2013-02-26 13:25:49]

*** Submitted:

Add API support for ServiceGet.

R=rog, TheMue
CC=
https://codereview.appspot.com/7384058