Code review comment for lp:~fwereade/juju-core/config-6-state-service-sane-methods

LGTM

https://codereview.appspot.com/10083047/diff/1/juju/conn.go
File juju/conn.go (right):

https://codereview.appspot.com/10083047/diff/1/juju/conn.go#newcode219
juju/conn.go:219: if err := svc.UpdateConfigSettings(settings); err !=
nil {
much nicer to only have one operation here, thanks.

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

https://codereview.appspot.com/10083047/diff/1/state/apiserver/client.go#newcode70
state/apiserver/client.go:70: changes, err :=
ch.Config().ParseSettingsYAML([]byte(p.Config), p.ServiceName)
we should check with the GUI folks that they haven't put in a workaround
for the current behaviour before this branch lands.

https://codereview.appspot.com/10083047/diff/1/state/service.go
File state/service.go (left):

https://codereview.appspot.com/10083047/diff/1/state/service.go#oldcode777
state/service.go:777: func (s *Service) SetConfigYAML(yamlData []byte)
error {
it's gone!

https://codereview.appspot.com/10083047/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/10083047/diff/1/state/service.go#newcode729
state/service.go:729: return settings.Map(), nil
i'd be tempted to return settings.core to avoid yet another unnecessary
map copy (or implement Settings.sharedMap which just returns
settings.core,
amounting to the same thing).

perhaps the overhead amounts to nothing in the long run though. <hits
self>.

https://codereview.appspot.com/10083047/diff/1/state/statecmd/get.go
File state/statecmd/get.go (right):

https://codereview.appspot.com/10083047/diff/1/state/statecmd/get.go#newcode52
state/statecmd/get.go:52: info := map[string]interface{}{
this should really be a struct, i think.

https://codereview.appspot.com/10083047/diff/1/worker/uniter/context_test.go
File worker/uniter/context_test.go (right):

https://codereview.appspot.com/10083047/diff/1/worker/uniter/context_test.go#newcode582
worker/uniter/context_test.go:582: })
c.Assert(err, IsNil)

https://codereview.appspot.com/10083047/