Code review comment for lp:~rogpeppe/juju-core/mfoord-wrapsingletonworkers

Revision history for this message
Roger Peppe (rogpeppe) wrote :

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()}
On 2014/04/10 11:14:38, nate.finch wrote:
> 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.

i don't we can do this, because newSingularRunner can fail, and if that
happens we don't want to leave the other jobs running.

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{
On 2014/04/10 11:14:38, nate.finch wrote:
> 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.

the slice is sorted (see the implementation of the started method)

https://codereview.appspot.com/86200043/

« Back to merge proposal