Code review comment for lp:~themue/juju-core/024-cleanup-watcher
Revision history for this message
| Frank Mueller (themue) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
Please take a look.
https:/
File state/state.go (right):
https:/
state/state.
On 2013/06/07 09:15:37, dimitern wrote:
> You don't need to define this, it's automatically available.
Done.
https:/
File state/state_test.go (right):
https:/
state/state_
On 2013/06/07 09:40:10, fwereade wrote:
> I think `defer stop(c, cw)` would be slightly more correct.
Done.
https:/
state/state_
s.AddTestingCha
On 2013/06/07 09:35:44, fwereade wrote:
> On 2013/06/07 09:15:37, dimitern wrote:
> > do you really need to do this twice? surely only one relation
followed by
> > destroy should be sufficient, but ymmv
> It'd support more sophisticated testing -- for event coalescing,
effect of
> Cleanup calls, etc -- quite nicely. Would be nice to see tests for
these cases.
Added some more and detected, that Cleanup() removes each document in an
own transaction which leads to more events. Will change this later in a
followup. Tests currently handle it.
https:/
File state/watcher.go (right):
https:/
state/watcher.
On 2013/06/07 09:15:37, dimitern wrote:
> in chan doesn't need to be a field of the watcher, you can create it
locally
> inside loop()
Done.
https:/
state/watcher.
On 2013/06/07 09:15:37, dimitern wrote:
> is there always an initial event that makes this line necessary? istm
this will
> always cause a notification to be sent, even when there's nothing
coming on the
> in chan.
It only sent the initial event, afterwards out has been nil and so no
event has been sent. When theres a signal via in out is changed to the
out channel again, sents one event and is set to nil again.
But I changed it to a simpler way.
https:/