Code review comment for lp:~niemeyer/juju-core/presence-polishing

Revision history for this message
William Reade (fwereade) wrote :

LGTM

https://codereview.appspot.com/6501114/diff/1/mstate/machine.go
File mstate/machine.go (right):

https://codereview.appspot.com/6501114/diff/1/mstate/machine.go#newcode114
mstate/machine.go:114: return fmt.Errorf("waiting for agent of machine
%v: watcher is dying", m)
On 2012/09/11 13:27:15, niemeyer wrote:
> On 2012/09/11 12:09:31, fwereade wrote:
> > ErrorContextf?

> Good catch, done.

> I've also refactored those methods a bit to reduce them.

Very neat :)

https://codereview.appspot.com/6501114/diff/1/mstate/presence/presence.go
File mstate/presence/presence.go (right):

https://codereview.appspot.com/6501114/diff/1/mstate/presence/presence.go#newcode240
mstate/presence/presence.go:240: // w.pending may get new requests as we
handle other requests.
On 2012/09/11 13:27:15, niemeyer wrote:
> On 2012/09/11 12:09:31, fwereade wrote:
> > I'm not sure I quite follow how this works. It seems that it's
specifically
> > *only* while handling requests that pending can change --

> Events dispatched from the syncing procedure go into pending too. I'm
happy to
> expand the comment, but do you have any suggestions that would make it
clarify
> the misunderstanding you have/had?

Everything I can think of is a more cumbersome rewording of what you
already have -- best just forget I said anything :)

https://codereview.appspot.com/6501114/

« Back to merge proposal