Code review comment for lp:~niemeyer/juju-core/mstate-machine-watcher

https://codereview.appspot.com/6489104/diff/5001/mstate/machine.go
File mstate/machine.go (right):

https://codereview.appspot.com/6489104/diff/5001/mstate/machine.go#newcode23
mstate/machine.go:23: TxnRevno int64 `bson:"txn-revno"`
On 2012/09/11 17:40:53, aram wrote:
> This complicates tests and watchers because Insert can't set TxnRevno
and
> DeepEquals will fail. How about reverting this, and create a new
machineDocTxn
> like this:

The reasoning for doing it doesn't sound great. Doing DeepEquals on such
a structure is most definitely wrong. Private data is private, and by
definition we should be able to cache information freely or change it
without breaking tests that work on the public APIs.

It sounds sensible to have a omitempty setting there, though, so that we
don't try to insert it. Can you please make the change on a follow up
CL?

https://codereview.appspot.com/6489104/diff/5001/mstate/watcher.go
File mstate/watcher.go (right):

https://codereview.appspot.com/6489104/diff/5001/mstate/watcher.go#newcode68
mstate/watcher.go:68: case w.changeChan <- m:
On 2012/09/11 17:40:53, aram wrote:
> Shouldn't this be a default case, and w.changeChan <- m *after* the
select? That
> way we will always completely drain ch before sending m. Now there's a
chance we
> will send m before draining ch.

No, there's some misunderstanding about how the watcher works. The
channel is never drained, and the construction here is purposefully
operating *while* trying to send.

https://codereview.appspot.com/6489104/