Merge into zookeeper : int32-to-int : Code : gozk

Status:	Merged
Merged at revision:	28
Proposed branch:	lp://staging/~niemeyer/gozk/int32-to-int
Merge into:	lp://staging/gozk/zookeeper
Diff against target:	287 lines (+39/-39) 3 files modified retry_test.go (+14/-14) zk.go (+13/-13) zk_test.go (+12/-12)
To merge this branch:	bzr merge lp://staging/~niemeyer/gozk/int32-to-int
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
The Go Language Gophers		2012-02-15	Pending
Review via email: mp+93252@code.staging.launchpad.net

Description of the change

Use int32 rather than int for stat fields.

https://codereview.appspot.com/5675059/

Revision history for this message

Gustavo Niemeyer (niemeyer) wrote on 2012-02-15:

#

Please take a look.

Revision history for this message

Gustavo Niemeyer (niemeyer) wrote on 2012-02-15:

#

Download full text (9.7 KiB)

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)
  }

...

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
=== gustavo@niemeyer.net-20120215134715-pd7hyip83ul5iahm >  
gustavo@niemeyer.net-20120215171834-jx3n0kc7dakgjx8j
=== 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
=== gustavo@niemeyer.net-20120215134715-pd7hyip83ul5iahm >  
gustavo@niemeyer.net-20120215171834-jx3n0kc7dakgjx8j
=== 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
=== gustavo@niemeyer.net-20120215134715-pd7hyip83ul5iahm >  
gustavo@niemeyer.net-20120215171834-jx3n0kc7dakgjx8j
=== 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)

Revision history for this message

William Reade (fwereade) wrote on 2012-02-15:

#

LGTM

https://codereview.appspot.com/5675059/

Revision history for this message

Roger Peppe (rogpeppe) wrote on 2012-02-15:

#

LGTM

https://codereview.appspot.com/5675059/

Revision history for this message

Gustavo Niemeyer (niemeyer) wrote on 2012-02-15:

#

*** Submitted:

Use int32 rather than int for stat fields.

R=fwereade, TheMue, rog
CC=
https://codereview.appspot.com/5675059

https://codereview.appspot.com/5675059/

gozk

Merge lp://staging/~niemeyer/gozk/int32-to-int into lp://staging/gozk/zookeeper

Commit message

Description of the change

Preview Diff

Subscribers