Code review comment for lp:~dimitern/juju-core/070-deployer-client-facade

Please take a look.

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go
File state/api/deployer/deployer_test.go (right):

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go#newcode53
state/api/deployer/deployer_test.go:53: s.stateAPI = s.OpenAPIAs(c,
s.machine.Tag(), "test-password")
On 2013/07/16 12:20:37, mue wrote:
> Stumbled upon this for the first time. You call it "login" in the
comment. So
> maybe LoginAsToAPI() would be better. But that's only an idea.

The idea AFAIU is that you always need to login when you open a
connection the the API server, hence the name of the function.

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go#newcode135
state/api/deployer/deployer_test.go:135: c.Assert(err, gc.IsNil)
On 2013/07/16 13:00:06, fwereade wrote:
> Removal shouldn't be necessary to see the change, fwiw.

Good point, will leave only the EnsureDead call.

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

https://codereview.appspot.com/11342043/diff/1/state/api/watcher/watcher.go#newcode179
state/api/watcher/watcher.go:179: stringWatcherId string
On 2013/07/16 12:20:37, mue wrote:
> It's StringsWatcher and StringsWatchId, so plural strings. Would name
the field
> here stringsWatcherId too.

Done.

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

https://codereview.appspot.com/11342043/diff/1/state/apiserver/root.go#newcode89
state/apiserver/root.go:89: // TODO: There is no direct test for this
On 2013/07/16 12:20:37, mue wrote:
> // TODO(dimitern) ...

Done.

https://codereview.appspot.com/11342043/diff/1/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/11342043/diff/1/state/machine.go#newcode163
state/machine.go:163: if !strings.HasPrefix(tag, machineTagPrefix) {
On 2013/07/16 12:20:37, mue wrote:
> How does code calling this function react when "" is returned? Maybe
it's better
> to return an error too.

> MachineIdFromTag(tag string) (string, error)

I didn't want to change the signature, but I added a test for that. I'll
add a TODO for your suggestion as a possible future improvement.

https://codereview.appspot.com/11342043/diff/1/state/unit.go
File state/unit.go (right):

https://codereview.appspot.com/11342043/diff/1/state/unit.go#newcode635
state/unit.go:635: if !strings.HasPrefix(tag, unitTagPrefix) {
On 2013/07/16 12:20:37, mue wrote:
> Same as said for machine here.

Same answer :)

https://codereview.appspot.com/11342043/