Thanks for the review Michi. I'll upload a new patch later today that fixes your comments.
On Mon, Oct 15, 2012 at 5:46 PM, <[email protected]> wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3781/#review12460 > ----------------------------------------------------------- > > > > /src/c/Makefile.am > <https://reviews.apache.org/r/3781/#comment26456> > > nitpick: a trailing space > > > > /src/c/src/zookeeper.c > <https://reviews.apache.org/r/3781/#comment26458> > > Do we need to call both srandom and srand48? > > > > /src/c/src/zookeeper.c > <https://reviews.apache.org/r/3781/#comment26459> > > typo: st to false -> set to false > > > > /src/c/src/zookeeper.c > <https://reviews.apache.org/r/3781/#comment26460> > > Is there a tab character here? > > > > /src/c/src/zookeeper.c > <https://reviews.apache.org/r/3781/#comment26462> > > Was the number 60 chosen arbitrarily? What happens if zh->recv_timeout > is less than 60? > > > > /src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml > <https://reviews.apache.org/r/3781/#comment26463> > > typo: and not jus -> and not just > > > I reviewed the C client part. Overall, it looks great! > > --Michi > > - michim > > > On Oct. 13, 2012, 7:56 a.m., Alexander Shraer wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/3781/ > > ----------------------------------------------------------- > > > > (Updated Oct. 13, 2012, 7:56 a.m.) > > > > > > Review request for zookeeper. > > > > > > Description > > ------- > > > > https://issues.apache.org/jira/browse/ZOOKEEPER-1355 > > > > > > Diffs > > ----- > > > > /src/c/Makefile.am 1397800 > > /src/c/include/zookeeper.h 1397800 > > /src/c/src/mt_adaptor.c 1397800 > > /src/c/src/st_adaptor.c 1397800 > > /src/c/src/zk_adaptor.h 1397800 > > /src/c/src/zookeeper.c 1397800 > > /src/c/tests/TestZookeeperClose.cc 1397800 > > /src/c/tests/TestZookeeperInit.cc 1397800 > > /src/c/tests/ZKMocks.cc 1397800 > > /src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml > 1397800 > > /src/java/main/org/apache/zookeeper/ZooKeeper.java 1397800 > > /src/java/main/org/apache/zookeeper/client/HostProvider.java 1397800 > > /src/java/main/org/apache/zookeeper/client/StaticHostProvider.java > 1397800 > > /src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java > 1397800 > > > > Diff: https://reviews.apache.org/r/3781/diff/ > > > > > > Testing > > ------- > > > > new tests included as part of the patch > > > > > > Thanks, > > > > Alexander Shraer > > > > > >
