LGTM with just a couple minor suggestions.
https://codereview.appspot.com/86200043/diff/40001/cmd/jujud/machine.go File cmd/jujud/machine.go (right):
https://codereview.appspot.com/86200043/diff/40001/cmd/jujud/machine.go#newcode253 cmd/jujud/machine.go:253: conn := singularAPIConn{st, st.Agent()} could this code get moved to the loop over entity.jobs below? It diesn't seem to neec to be here, and it puts the declaration far from the use of the variable.
https://codereview.appspot.com/86200043/diff/40001/cmd/jujud/machine_test.go File cmd/jujud/machine_test.go (right):
https://codereview.appspot.com/86200043/diff/40001/cmd/jujud/machine_test.go#newcode384 cmd/jujud/machine_test.go:384: c.Assert(s.singularRecord.started(), jc.DeepEquals, []string{ This checks order of the slice, too, right? Does the order matter? If not, we may want to use jc.SameContents here, so we don't fail the test if the order changes.
https://codereview.appspot.com/86200043/
« Back to merge proposal
LGTM with just a couple minor suggestions.
https:/ /codereview. appspot. com/86200043/ diff/40001/ cmd/jujud/ machine. go machine. go (right):
File cmd/jujud/
https:/ /codereview. appspot. com/86200043/ diff/40001/ cmd/jujud/ machine. go#newcode253 machine. go:253: conn := singularAPIConn{st, st.Agent()}
cmd/jujud/
could this code get moved to the loop over entity.jobs below? It
diesn't seem to neec to be here, and it puts the declaration far from
the use of the variable.
https:/ /codereview. appspot. com/86200043/ diff/40001/ cmd/jujud/ machine_ test.go machine_ test.go (right):
File cmd/jujud/
https:/ /codereview. appspot. com/86200043/ diff/40001/ cmd/jujud/ machine_ test.go# newcode384 machine_ test.go: 384: c.Assert( s.singularRecor d.started( ),
cmd/jujud/
jc.DeepEquals, []string{
This checks order of the slice, too, right? Does the order matter? If
not, we may want to use jc.SameContents here, so we don't fail the test
if the order changes.
https:/ /codereview. appspot. com/86200043/