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

Revision history for this message
William Reade (fwereade) wrote :

LGTM. I can see arguments both ways for the *FromTag functions... I
think it's probably good enough that they return things that are never
valid ids/names.

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#newcode135
state/api/deployer/deployer_test.go:135: c.Assert(err, gc.IsNil)
Removal shouldn't be necessary to see the change, fwiw.

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.

+1

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

« Back to merge proposal