Code review comment for lp:~dave-cheney/juju-core/go-cmd-juju-add-unit

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

Code looks fine, but:

https://codereview.appspot.com/6449093/diff/4001/cmd/juju/addunit.go
File cmd/juju/addunit.go (right):

https://codereview.appspot.com/6449093/diff/4001/cmd/juju/addunit.go#newcode1
cmd/juju/addunit.go:1: package main
Can we call this add-unit please? Non-matching names just bug me :).

https://codereview.appspot.com/6449093/diff/4001/cmd/juju/addunit.go#newcode51
cmd/juju/addunit.go:51: st, err := conn.State()
I *think* that this is Doing It Wrong, and that Conn clients are not
meant to interact with state directly, but I may have the wrong end of
the stick.

https://codereview.appspot.com/6449093/

« Back to merge proposal