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

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

Please take a look.

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 {
On 2013/03/07 18:05:24, dimitern wrote:
> doc comment?

Done.

https://codereview.appspot.com/7563046/diff/1/state/api/apiclient.go#newcode137
state/api/apiclient.go:137: return &AllWatcher{
On 2013/03/07 18:05:24, dimitern wrote:
> you need just &AllWatcher{client, id} here.

Done.

https://codereview.appspot.com/7563046/diff/1/state/api/apiclient.go#newcode143
state/api/apiclient.go:143: func (watcher *AllWatcher) Next()
(*[]params.Delta, error) {
On 2013/03/07 18:05:24, dimitern wrote:
> why not return just []params.Delta, error ? nil is a valid slice
value, if the
> pointer needs to express missing params.

Done.

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 {
On 2013/03/07 18:05:24, dimitern wrote:
> // UnmarshalJSON implements json.Marshaller. ?

Done.

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)
On 2013/03/07 18:05:24, dimitern wrote:
> s/%#v/%q/ ?

Done.

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)
On 2013/03/07 18:05:24, dimitern wrote:
> ditto

Done.

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

« Back to merge proposal