A few comments.
https://codereview.appspot.com/10078043/diff/6001/state/state.go File state/state.go (right):
https://codereview.appspot.com/10078043/diff/6001/state/state.go#newcode1079 state/state.go:1079: TxnRevno int64 `bson:"txn-revno"` You don't need to define this, it's automatically available.
https://codereview.appspot.com/10078043/diff/6001/state/state_test.go File state/state_test.go (right):
https://codereview.appspot.com/10078043/diff/6001/state/state_test.go#newcode1362 state/state_test.go:1362: case _, ok := <-cw.Changes(): Please add c.Assert(ok, Equals, true), otherwise this is not a change notification.
https://codereview.appspot.com/10078043/diff/6001/state/state_test.go#newcode1390 state/state_test.go:1390: _, err = s.State.AddService("varnish", s.AddTestingCharm(c, "varnish")) do you really need to do this twice? surely only one relation followed by destroy should be sufficient, but ymmv
https://codereview.appspot.com/10078043/diff/6001/state/watcher.go File state/watcher.go (right):
https://codereview.appspot.com/10078043/diff/6001/state/watcher.go#newcode1256 state/watcher.go:1256: in chan watcher.Change in chan doesn't need to be a field of the watcher, you can create it locally inside loop()
https://codereview.appspot.com/10078043/diff/6001/state/watcher.go#newcode1288 state/watcher.go:1288: out := w.out 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.
https://codereview.appspot.com/10078043/
« Back to merge proposal
A few comments.
https:/ /codereview. appspot. com/10078043/ diff/6001/ state/state. go
File state/state.go (right):
https:/ /codereview. appspot. com/10078043/ diff/6001/ state/state. go#newcode1079 go:1079: TxnRevno int64 `bson:"txn-revno"`
state/state.
You don't need to define this, it's automatically available.
https:/ /codereview. appspot. com/10078043/ diff/6001/ state/state_ test.go
File state/state_test.go (right):
https:/ /codereview. appspot. com/10078043/ diff/6001/ state/state_ test.go# newcode1362 test.go: 1362: case _, ok := <-cw.Changes():
state/state_
Please add c.Assert(ok, Equals, true), otherwise this is not a change
notification.
https:/ /codereview. appspot. com/10078043/ diff/6001/ state/state_ test.go# newcode1390 test.go: 1390: _, err = s.State. AddService( "varnish" , rm(c, "varnish"))
state/state_
s.AddTestingCha
do you really need to do this twice? surely only one relation followed
by destroy should be sufficient, but ymmv
https:/ /codereview. appspot. com/10078043/ diff/6001/ state/watcher. go
File state/watcher.go (right):
https:/ /codereview. appspot. com/10078043/ diff/6001/ state/watcher. go#newcode1256 go:1256: in chan watcher.Change
state/watcher.
in chan doesn't need to be a field of the watcher, you can create it
locally inside loop()
https:/ /codereview. appspot. com/10078043/ diff/6001/ state/watcher. go#newcode1288 go:1288: out := w.out
state/watcher.
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.
https:/ /codereview. appspot. com/10078043/