gozk

Code review comment for lp:~niemeyer/gozk/int32-to-int

  1. int32-to-int
  2. Merge into zookeeper
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Reviewers: mp+93252_code.launchpad.net,

Message:
Please take a look.

Description:

https://code.launchpad.net/~niemeyer/gozk/int32-to-int/+merge/93252

(do not edit description out of merge proposal)

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

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

Index: retry_test.go
=== <email address hidden> >
<email address hidden>
=== modified file 'retry_test.go'
--- retry_test.go 2011-11-29 17:44:18 +0000
+++ retry_test.go 2012-02-15 17:18:34 +0000
@@ -20,7 +20,7 @@
   data, stat, err := conn.Get("/test")
   c.Assert(err, IsNil)
   c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)
   c.Assert(data, Equals, "new")

   acl, _, err := conn.ACL("/test")
@@ -38,7 +38,7 @@
    func(data string, stat *zk.Stat) (string, error) {
     c.Assert(data, Equals, "old")
     c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)
     return "brand new", nil
    })
   c.Assert(err, IsNil)
@@ -46,7 +46,7 @@
   data, stat, err := conn.Get("/test")
   c.Assert(err, IsNil)
   c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(1))
+ c.Assert(stat.Version(), Equals, 1)
   c.Assert(data, Equals, "brand new")

   // ACL was unchanged by RetryChange().
@@ -65,7 +65,7 @@
    func(data string, stat *zk.Stat) (string, error) {
     c.Assert(data, Equals, "old")
     c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)
     return "old", nil
    })
   c.Assert(err, IsNil)
@@ -73,7 +73,7 @@
   data, stat, err := conn.Get("/test")
   c.Assert(err, IsNil)
   c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0)) // Unchanged!
+ c.Assert(stat.Version(), Equals, 0) // Unchanged!
   c.Assert(data, Equals, "old")
  }

@@ -90,7 +90,7 @@
     return "<none> => conflict", nil
    case "conflict":
     c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)
     return "conflict => new", nil
    default:
     c.Fatal("Unexpected node data: " + data)
@@ -105,7 +105,7 @@
   c.Assert(err, IsNil)
   c.Assert(data, Equals, "conflict => new")
   c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(1))
+ c.Assert(stat.Version(), Equals, 1)
  }

  func (s *S) TestRetryChangeConflictOnSetDueToChange(c *C) {
@@ -118,13 +118,13 @@
    switch data {
    case "old":
     c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)
     _, err := conn.Set("/test", "conflict", 0)
     c.Assert(err, IsNil)
     return "old => new", nil
    case "conflict":
     c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(1))
+ c.Assert(stat.Version(), Equals, 1)
     return "conflict => new", nil
    default:
     c.Fatal("Unexpected node data: " + data)
@@ -139,7 +139,7 @@
   c.Assert(err, IsNil)
   c.Assert(data, Equals, "conflict => new")
   c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(2))
+ c.Assert(stat.Version(), Equals, 2)
  }

  func (s *S) TestRetryChangeConflictOnSetDueToDelete(c *C) {
@@ -152,7 +152,7 @@
    switch data {
    case "old":
     c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)
     err := conn.Delete("/test", 0)
     c.Assert(err, IsNil)
     return "old => <deleted>", nil
@@ -172,7 +172,7 @@
   c.Assert(err, IsNil)
   c.Assert(data, Equals, "<deleted> => new")
   c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)

   // Should be the new ACL.
   acl, _, err := conn.ACL("/test")
@@ -214,7 +214,7 @@
   stat, err := conn.Exists("/test")
   c.Assert(err, IsNil)
   c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)

   c.Assert(called, Equals, false)
  }
@@ -237,7 +237,7 @@
   stat, err := conn.Exists("/test")
   c.Assert(err, IsNil)
   c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)

   c.Assert(called, Equals, true)
  }

Index: zk.go
=== <email address hidden> >
<email address hidden>
=== modified file 'zk.go'
--- zk.go 2012-02-13 14:47:46 +0000
+++ zk.go 2012-02-15 17:18:34 +0000
@@ -331,19 +331,19 @@
  }

  // Version returns the number of changes to the data of the node.
-func (stat *Stat) Version() int32 {
- return int32(stat.c.version)
+func (stat *Stat) Version() int {
+ return int(stat.c.version)
  }

  // CVersion returns the number of changes to the children of the node.
  // This only changes when children are created or removed.
-func (stat *Stat) CVersion() int32 {
- return int32(stat.c.cversion)
+func (stat *Stat) CVersion() int {
+ return int(stat.c.cversion)
  }

  // AVersion returns the number of changes to the ACL of the node.
-func (stat *Stat) AVersion() int32 {
- return int32(stat.c.aversion)
+func (stat *Stat) AVersion() int {
+ return int(stat.c.aversion)
  }

  // If the node is an ephemeral node, EphemeralOwner returns the session id
@@ -353,13 +353,13 @@
  }

  // DataLength returns the length of the data in the node in bytes.
-func (stat *Stat) DataLength() int32 {
- return int32(stat.c.dataLength)
+func (stat *Stat) DataLength() int {
+ return int(stat.c.dataLength)
  }

  // NumChildren returns the number of children of the node.
-func (stat *Stat) NumChildren() int32 {
- return int32(stat.c.numChildren)
+func (stat *Stat) NumChildren() int {
+ return int(stat.c.numChildren)
  }

  // Pzxid returns the Pzxid of the node, whatever that is.
@@ -657,7 +657,7 @@
  //
  // It is an error to attempt to set the data of a non-existing node with
  // this function. In these cases, use Create instead.
-func (conn *Conn) Set(path, value string, version int32) (stat *Stat, err
error) {
+func (conn *Conn) Set(path, value string, version int) (stat *Stat, err
error) {

   cpath := C.CString(path)
   cvalue := C.CString(value)
@@ -677,7 +677,7 @@
  // Delete removes the node at path. If version is not -1, the operation
  // will only succeed if the node is still at this version when the
  // node is deleted as an atomic operation.
-func (conn *Conn) Delete(path string, version int32) (err error) {
+func (conn *Conn) Delete(path string, version int) (err error) {
   cpath := C.CString(path)
   defer C.free(unsafe.Pointer(cpath))
   rc, cerr := C.zoo_delete(conn.handle, cpath, C.int(version))
@@ -732,7 +732,7 @@
  }

  // SetACL changes the access control list for path.
-func (conn *Conn) SetACL(path string, aclv []ACL, version int32) error {
+func (conn *Conn) SetACL(path string, aclv []ACL, version int) error {

   cpath := C.CString(path)
   defer C.free(unsafe.Pointer(cpath))

Index: zk_test.go
=== <email address hidden> >
<email address hidden>
=== modified file 'zk_test.go'
--- zk_test.go 2012-02-13 14:47:46 +0000
+++ zk_test.go 2012-02-15 17:18:34 +0000
@@ -142,12 +142,12 @@
   c.Assert(stat.Mzxid(), Equals, int64(0))
   c.Assert(stat.CTime(), Equals, time.Unix(0, 0))
   c.Assert(stat.MTime(), Equals, time.Unix(0, 0))
- c.Assert(stat.Version(), Equals, int32(0))
- c.Assert(stat.CVersion(), Equals, int32(0))
- c.Assert(stat.AVersion(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)
+ c.Assert(stat.CVersion(), Equals, 0)
+ c.Assert(stat.AVersion(), Equals, 0)
   c.Assert(stat.EphemeralOwner(), Equals, int64(0))
- c.Assert(stat.DataLength(), Equals, int32(0))
- c.Assert(stat.NumChildren(), Equals, int32(1))
+ c.Assert(stat.DataLength(), Equals, 0)
+ c.Assert(stat.NumChildren(), Equals, 1)
   c.Assert(stat.Pzxid(), Equals, int64(0))
  }

@@ -201,7 +201,7 @@
   start = time.Now()
   stat, err = conn.Set("/test", "bababum", -1) // Any version.
   c.Assert(err, IsNil)
- c.Assert(stat.Version(), Equals, int32(1))
+ c.Assert(stat.Version(), Equals, 1)
   checkTimeBetween(c, "mtime", stat.MTime(), start, time.Now())

   data, _, err := conn.Get("/test")
@@ -222,7 +222,7 @@
   data, stat, watch, err := conn.GetW("/test")
   c.Assert(err, IsNil)
   c.Assert(data, Equals, "one")
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)

   select {
   case <-watch:
@@ -313,7 +313,7 @@
   children, stat, err := conn.Children("/")
   c.Assert(err, IsNil)
   c.Assert(children, Equals, []string{"zookeeper"})
- c.Assert(stat.NumChildren(), Equals, int32(1))
+ c.Assert(stat.NumChildren(), Equals, 1)

   children, stat, err = conn.Children("/non-existent")
   c.Assert(err, Equals, zk.ZNONODE)
@@ -331,7 +331,7 @@
   children, stat, watch, err := conn.ChildrenW("/")
   c.Assert(err, IsNil)
   c.Assert(children, Equals, []string{"zookeeper"})
- c.Assert(stat.NumChildren(), Equals, int32(1))
+ c.Assert(stat.NumChildren(), Equals, 1)

   select {
   case <-watch:
@@ -352,7 +352,7 @@

   children, stat, watch, err = conn.ChildrenW("/")
   c.Assert(err, IsNil)
- c.Assert(stat.NumChildren(), Equals, int32(2))
+ c.Assert(stat.NumChildren(), Equals, 2)

   // The ordering is most likely unstable, so this test must be fixed.
   c.Assert(children, Equals, []string{"test1", "zookeeper"})
@@ -432,7 +432,7 @@
   stat, watch, err = conn.ExistsW("/test")
   c.Assert(err, IsNil)
   c.Assert(stat, NotNil)
- c.Assert(stat.NumChildren(), Equals, int32(0))
+ c.Assert(stat.NumChildren(), Equals, 0)

   c.Check(zk.CountPendingWatches(), Equals, 2)
  }
@@ -513,7 +513,7 @@
   c.Assert(err, IsNil)
   c.Assert(acl, Equals, zk.WorldACL(zk.PERM_ALL))
   c.Assert(stat, NotNil)
- c.Assert(stat.Version(), Equals, int32(0))
+ c.Assert(stat.Version(), Equals, 0)

   acl, stat, err = conn.ACL("/non-existent")
   c.Assert(err, NotNil)

« Back to merge proposal