Merge lp:~rogpeppe/gozk/zk-error-fixes into lp:~juju/gozk/zk

Proposed by Roger Peppe on 2011-11-29
Status: Merged
Merged at revision: 22
Proposed branch: lp:~rogpeppe/gozk/zk-error-fixes
Merge into: lp:~juju/gozk/zk
Diff against target: 0 lines
To merge this branch: bzr merge lp:~rogpeppe/gozk/zk-error-fixes
Reviewer Review Type Date Requested Status
Juju Engineering 2011-11-29 Pending
Review via email: mp+83816@code.launchpad.net

Description of the change

gozk/zk: fixes for new error type

gozk/zk: fixes for new error type

https://codereview.appspot.com/5440056/

To post a comment you must log in.
lp:~rogpeppe/gozk/zk-error-fixes updated on 2011-11-29
22. By Roger Peppe on 2011-11-29

fixes for new error type

Gustavo Niemeyer (niemeyer) wrote :

Beautiful, LGTM, with one detail:

https://codereview.appspot.com/5440056/diff/1/zk_test.go
File zk_test.go (right):

https://codereview.appspot.com/5440056/diff/1/zk_test.go#newcode305
zk_test.go:305: c.Assert(len(children), Equals, 0)
IsNil?

https://codereview.appspot.com/5440056/

Roger Peppe (rogpeppe) wrote :

Reviewers: mp+83816_code.launchpad.net, niemeyer,

https://codereview.appspot.com/5440056/diff/1/zk_test.go
File zk_test.go (right):

https://codereview.appspot.com/5440056/diff/1/zk_test.go#newcode305
zk_test.go:305: c.Assert(len(children), Equals, 0)
On 2011/11/29 17:32:33, niemeyer wrote:
> IsNil?

yeah. and the NotNil error assertion is probably not necessary either.

Description:
gozk/zk: fixes for new error type

https://code.launchpad.net/~rogpeppe/gozk/zk-error-fixes/+merge/83816

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/5440056/

Affected files:
   M retry_test.go
   M server.go
   M suite_test.go
   M zk.go
   M zk_test.go

lp:~rogpeppe/gozk/zk-error-fixes updated on 2011-11-29
23. By Roger Peppe on 2011-11-29

minor fix to TestChildren

Preview Diff

Empty

Subscribers

People subscribed via source and target branches

to all changes: