Code review comment for lp://staging/~gary/juju-core/megawatcher_api

Revision history for this message
Gary Poster (gary) wrote :

*** Submitted:

Exposed as-yet-unimplemented state watcher

Add api server code, api client code, and partly stub tests. Delta code
had to be moved to the new params location.

R=dimitern, rog
CC=
https://codereview.appspot.com/7563046

https://codereview.appspot.com/7563046/diff/6001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/7563046/diff/6001/state/api/params/params.go#newcode68
state/api/params/params.go:68: type AllWatcherNext struct {
On 2013/03/07 21:52:03, rog wrote:
> i've been trying to use the convention that params have no suffix, but
results
> have a Result suffix.

> so i'd name this AllWatcherNextResult.
> long but consistent :-)

As we discussed, we used "...Results" instead to match the other one.

https://codereview.appspot.com/7563046/diff/6001/state/api/params/params.go#newcode91
state/api/params/params.go:91: type Delta struct {
On 2013/03/07 21:52:03, rog wrote:
> looks like the branch with this code should have been a prereq

As we discussed, this was a code move because of a conflict with some
changes in master.

https://codereview.appspot.com/7563046/diff/6001/state/apiserver/api_test.go
File state/apiserver/api_test.go (right):

https://codereview.appspot.com/7563046/diff/6001/state/apiserver/api_test.go#newcode1000
state/apiserver/api_test.go:1000: stopped := false
On 2013/03/07 21:52:03, rog wrote:
> rather than having the bool, we could probably do:
> defer func)() {
> err := watcher.Stop()
> c.Check(err, IsNil)
> }()

> and lose the Stop at the end of the function (and its comment)

Done.

https://codereview.appspot.com/7563046/

« Back to merge proposal