----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2313/#review2474 -----------------------------------------------------------
/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java <https://reviews.apache.org/r/2313/#comment5591> Should throw an Exception instead of returning an error code. /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java <https://reviews.apache.org/r/2313/#comment5588> Should log the error code here. /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java <https://reviews.apache.org/r/2313/#comment5589> The parseConfig without version is no longer used. It should just be removed. /bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ConditionalSetTest.java <https://reviews.apache.org/r/2313/#comment5592> Indentation looks a bit funny here. The coding convention is a bit ambiguous on this, but i'd indent 88-91 by 8 spaces each. /bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ConditionalSetTest.java <https://reviews.apache.org/r/2313/#comment5590> You could use assertEquals("Was able to write to ZooKeeper", 0, rc); here, it automatically prints out both the numbers needed. - Ivan On 2011-10-08 13:03:41, fpj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2313/ > ----------------------------------------------------------- > > (Updated 2011-10-08 13:03:41) > > > Review request for bookkeeper and Ivan Kelly. > > > Summary > ------- > > Patch for BookKeeper-68. > > > Diffs > ----- > > > /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java > 1172822 > > /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java > 1172822 > > /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java > 1172822 > > /bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java > 1172822 > > /bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ConditionalSetTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/2313/diff > > > Testing > ------- > > > Thanks, > > fpj > >
