LGTM modulo the suggestions below. Nice simplification with the clientError() stuff!
https://codereview.appspot.com/7563046/diff/1/state/api/apiclient.go File state/api/apiclient.go (right):
https://codereview.appspot.com/7563046/diff/1/state/api/apiclient.go#newcode131 state/api/apiclient.go:131: type AllWatcher struct { doc comment?
https://codereview.appspot.com/7563046/diff/1/state/api/apiclient.go#newcode137 state/api/apiclient.go:137: return &AllWatcher{ you need just &AllWatcher{client, id} here.
https://codereview.appspot.com/7563046/diff/1/state/api/apiclient.go#newcode143 state/api/apiclient.go:143: func (watcher *AllWatcher) Next() (*[]params.Delta, error) { why not return just []params.Delta, error ? nil is a valid slice value, if the pointer needs to express missing params.
https://codereview.appspot.com/7563046/diff/1/state/api/params/params.go File state/api/params/params.go (right):
https://codereview.appspot.com/7563046/diff/1/state/api/params/params.go#newcode117 state/api/params/params.go:117: func (d *Delta) UnmarshalJSON(data []byte) error { // UnmarshalJSON implements json.Marshaller. ?
https://codereview.appspot.com/7563046/diff/1/state/api/params/params.go#newcode137 state/api/params/params.go:137: return fmt.Errorf("Unexpected operation %#v", operation) s/%#v/%q/ ?
https://codereview.appspot.com/7563046/diff/1/state/api/params/params.go#newcode149 state/api/params/params.go:149: return fmt.Errorf("Unexpected entity name %#v", entityKind) ditto
https://codereview.appspot.com/7563046/diff/1/state/api/params/params_test.go File state/api/params/params_test.go (right):
https://codereview.appspot.com/7563046/diff/1/state/api/params/params_test.go#newcode105 state/api/params/params_test.go:105: c.Check( it'll be nicer to read the code if you assign the json.Unmarshal() to a var to check (all 3 cases below).
https://codereview.appspot.com/7563046/
« Back to merge proposal
LGTM modulo the suggestions below. Nice simplification with the
clientError() stuff!
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ apiclient. go apiclient. go (right):
File state/api/
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ apiclient. go#newcode131 apiclient. go:131: type AllWatcher struct {
state/api/
doc comment?
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ apiclient. go#newcode137 apiclient. go:137: return &AllWatcher{
state/api/
you need just &AllWatcher{client, id} here.
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ apiclient. go#newcode143 apiclient. go:143: func (watcher *AllWatcher) Next()
state/api/
(*[]params.Delta, error) {
why not return just []params.Delta, error ? nil is a valid slice value,
if the pointer needs to express missing params.
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ params/ params. go params/ params. go (right):
File state/api/
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ params/ params. go#newcode117 params/ params. go:117: func (d *Delta) UnmarshalJSON(data
state/api/
[]byte) error {
// UnmarshalJSON implements json.Marshaller. ?
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ params/ params. go#newcode137 params/ params. go:137: return fmt.Errorf( "Unexpected operation
state/api/
%#v", operation)
s/%#v/%q/ ?
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ params/ params. go#newcode149 params/ params. go:149: return fmt.Errorf( "Unexpected entity
state/api/
name %#v", entityKind)
ditto
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ params/ params_ test.go params/ params_ test.go (right):
File state/api/
https:/ /codereview. appspot. com/7563046/ diff/1/ state/api/ params/ params_ test.go# newcode105 params/ params_ test.go: 105: c.Check(
state/api/
it'll be nicer to read the code if you assign the json.Unmarshal() to a
var to check (all 3 cases below).
https:/ /codereview. appspot. com/7563046/