Code review comment for lp:~themue/juju-core/024-cleanup-watcher

Sorry, NOT LGTM. This does not do what we agreed (notify on any change
to the cleanups collection), does stuff we don't want (notifies even if
there are no changes to the cleanups collection) is not self-consistent
(an empty collection sometimes generates an event, and sometimes does
not)... and is *not a watcher*, either in form or in function.

https://codereview.appspot.com/10078043/diff/1/state/watcher.go
File state/watcher.go (right):

https://codereview.appspot.com/10078043/diff/1/state/watcher.go#newcode1244
state/watcher.go:1244: // After a unit is found to be Dead, no further
event will include it.
Fix these please :).

https://codereview.appspot.com/10078043/diff/1/state/watcher.go#newcode1283
state/watcher.go:1283: next := time.After(watcher.Period)
Why is this not a... watcher? We discussed this yesterday: there's no
call whatsoever to make this periodic. We can just use the usual,
conventional mechanism of watching the st.cleanups collection, but with
the additional simplification that I'm happy just sending out a
notification whenever there's any change whatsoever to any document in
there.

I really tried to be as explicit as possible; what do you think is the
source of the communication problems here?

https://codereview.appspot.com/10078043/