worker: Deployer/MinUnitsWorker use StringsWorker

worker: Deployer/MinUnitsWorker use StringsWorker

Both MinUnitsWorker and Deployer now use the new
StringsWorker. Code refactored and simplified,
no other changes. Live tested on EC2.

https://code.launchpad.net/~dimitern/juju-core/085-use-stringsworker/+merge/178073

Requires: https://code.launchpad.net/~dimitern/juju-core/084-stringsworker/+merge/178032

(do not edit description out of merge proposal)

[dimitern, 2013-08-01 14:05:27]

Please take a look.

[mue, 2013-08-01 14:10:19]

LGTM, looks cleaner now. New approach is really good.

[rog, 2013-08-01 14:49:26]

LGTM, although still unconvinced that the line
count reduction is worth the less obvious control flow.

[gz, 2013-08-01 15:00:32]

LGTM

https://codereview.appspot.com/12254043/diff/1/worker/deployer/deployer.go
File worker/deployer/deployer.go (right):

https://codereview.appspot.com/12254043/diff/1/worker/deployer/deployer.go#ne...
worker/deployer/deployer.go:66: func (d *Deployer) SetUp() (api.StringsWatcher,
error) {
The logic in here is a little non-obvious to me, but it's just the same as the
lead section of the old loop function, so fair enough.

https://codereview.appspot.com/12254043/diff/1/worker/minunitsworker/minunits...
File worker/minunitsworker/minunitsworker_test.go (right):

https://codereview.appspot.com/12254043/diff/1/worker/minunitsworker/minunits...
worker/minunitsworker/minunitsworker_test.go:35: defer func() {
c.Assert(worker.Stop(mu), gc.IsNil) }()
Seems like the old spelling would have kept working anyway, as it just takes a
Stopper interface?

[dimitern, 2013-08-01 15:42:21]

Please take a look.

https://codereview.appspot.com/12254043/diff/1/worker/deployer/deployer.go
File worker/deployer/deployer.go (right):

https://codereview.appspot.com/12254043/diff/1/worker/deployer/deployer.go#ne...
worker/deployer/deployer.go:66: func (d *Deployer) SetUp() (api.StringsWatcher,
error) {
On 2013/08/01 15:00:33, gz wrote:
> The logic in here is a little non-obvious to me, but it's just the same as the
> lead section of the old loop function, so fair enough.

Yeah, exactly - what used to be done before the main loop in loop() is done here
now. Take a look at stringsworker.go - the loop().

https://codereview.appspot.com/12254043/diff/1/worker/minunitsworker/minunits...
File worker/minunitsworker/minunitsworker_test.go (right):

https://codereview.appspot.com/12254043/diff/1/worker/minunitsworker/minunits...
worker/minunitsworker/minunitsworker_test.go:35: defer func() {
c.Assert(worker.Stop(mu), gc.IsNil) }()
On 2013/08/01 15:00:33, gz wrote:
> Seems like the old spelling would have kept working anyway, as it just takes a
> Stopper interface?

Not really, because now the worker.Worker is used, and it doesn't have a Stop()
method, instead worker.Stop(w) is used, which internally calls w.Kill and
w.Wait.