Code review comment for lp://staging/~rogpeppe/gozk/update-server-interface

Revision history for this message
Roger Peppe (rogpeppe) wrote :

On 5 October 2011 15:49, Gustavo Niemeyer <email address hidden> wrote:
> [10]
>
>
> +// exist, it assumes the server is not running and returns (nil, nil).
>
> This interface is strange, and can easily lead to bugs since it's not an idiom in Go.
> I'd never assume p is nil if err == nil. Please return an error if p == nil.

i've changed this, but for reference, i've just noticed a similar example
in your code, in Exists:

 // We diverge a bit from the usual here: a ZNONODE is not an error
 // for an exists call, otherwise every Exists call would have to check
 // for err != nil and err.Code() != ZNONODE.

it's for a very similar reason that i returned a nil process and nil error from
Process when there was no process to be found.

« Back to merge proposal