LGTM with one or two trivials below. thanks!
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 { 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 :-)
https://codereview.appspot.com/7563046/diff/6001/state/api/params/params.go#newcode91 state/api/params/params.go:91: type Delta struct { looks like the branch with this code should have been a prereq
https://codereview.appspot.com/7563046/diff/6001/state/api/params/params_test.go File state/api/params/params_test.go (right):
https://codereview.appspot.com/7563046/diff/6001/state/api/params/params_test.go#newcode10 state/api/params/params_test.go:10: // TestPackage integrates the tests into gotest. same here.
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 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)
https://codereview.appspot.com/7563046/diff/6001/state/megawatcher.go File state/megawatcher.go (right):
https://codereview.appspot.com/7563046/diff/6001/state/megawatcher.go#newcode25 state/megawatcher.go:25: var StubNextDelta = []params.Delta{ s/Stub/stub/ (or do you want it accessible from tests outside state?)
https://codereview.appspot.com/7563046/
« Back to merge proposal
LGTM with one or two trivials below. thanks!
https:/ /codereview. appspot. com/7563046/ diff/6001/ state/api/ params/ params. go params/ params. go (right):
File state/api/
https:/ /codereview. appspot. com/7563046/ diff/6001/ state/api/ params/ params. go#newcode68 params/ params. go:68: type AllWatcherNext struct {
state/api/
i've been trying to use the convention that params have no suffix, but
results have a Result suffix.
so i'd name this AllWatcherNextR esult.
long but consistent :-)
https:/ /codereview. appspot. com/7563046/ diff/6001/ state/api/ params/ params. go#newcode91 params/ params. go:91: type Delta struct {
state/api/
looks like the branch with this code should have been a prereq
https:/ /codereview. appspot. com/7563046/ diff/6001/ state/api/ params/ params_ test.go params/ params_ test.go (right):
File state/api/
https:/ /codereview. appspot. com/7563046/ diff/6001/ state/api/ params/ params_ test.go# newcode10 params/ params_ test.go: 10: // TestPackage integrates the tests
state/api/
into gotest.
same here.
https:/ /codereview. appspot. com/7563046/ diff/6001/ state/apiserver /api_test. go /api_test. go (right):
File state/apiserver
https:/ /codereview. appspot. com/7563046/ diff/6001/ state/apiserver /api_test. go#newcode1000 /api_test. go:1000: stopped := false
state/apiserver
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)
https:/ /codereview. appspot. com/7563046/ diff/6001/ state/megawatch er.go er.go (right):
File state/megawatch
https:/ /codereview. appspot. com/7563046/ diff/6001/ state/megawatch er.go#newcode25 er.go:25: var StubNextDelta = []params.Delta{
state/megawatch
s/Stub/stub/
(or do you want it accessible from tests outside state?)
https:/ /codereview. appspot. com/7563046/