-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23081/#review46875
-----------------------------------------------------------


It looks good to me, I just have a few minor comments, and I need to finish 
checking the test cases. Also, it would be great if you could remove the 
additional spaces (red marks in the diff).


src/c/README
<https://reviews.apache.org/r/23081/#comment82441>

    This is minor, but does the option really have to be at the end? I'd say 
that either anywhere or before the host:port pair, no?



src/c/src/zookeeper.c
<https://reviews.apache.org/r/23081/#comment82442>

    Let's use _WIN32 instead.



src/c/src/zookeeper.c
<https://reviews.apache.org/r/23081/#comment82443>

    this is commented out, should we just remove it here?



src/c/src/zookeeper.c
<https://reviews.apache.org/r/23081/#comment82444>

    I'm not sure I'm following this part of the code. here we just spotted that 
we are in ro mode but shouldn't because we found an rw server. what does 
handle_error do about that?



src/c/tests/ZKMocks.cc
<https://reviews.apache.org/r/23081/#comment82448>

    spaces.



src/c/tests/zkServer.sh
<https://reviews.apache.org/r/23081/#comment82449>

    why are you calling startPeer the mode with ro enabled? startReadOnly seems 
more clear, no?


- fpj


On June 26, 2014, 7:09 p.m., Raul Gutierrez Segales wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23081/
> -----------------------------------------------------------
> 
> (Updated June 26, 2014, 7:09 p.m.)
> 
> 
> Review request for zookeeper, fpj, michim, Patrick Hunt, Rakesh R, and Thawan 
> Kooburat.
> 
> 
> Bugs: ZOOKEEPER-827
>     https://issues.apache.org/jira/browse/ZOOKEEPER-827
> 
> 
> Repository: zookeeper-git
> 
> 
> Description
> -------
> 
> commit 86a75dcaa67e099f8c5722f5190c601663e44c75
> Author: Raul Gutierrez S <r...@itevenworks.net>
> Date:   Thu Jun 26 11:19:11 2014 -0700
> 
>     ZOOKEEPER-827. enable r/o mode in C client library
>     
>     Signed-off-by: Raul Gutierrez S <r...@itevenworks.net>
> 
> :100644 100644 6130c4d... a7b2621... M        src/c/Makefile.am
> :100644 100644 fe25015... b45ca0f... M        src/c/README
> :100644 100644 d5a3876... 18a203d... M        src/c/include/zookeeper.h
> :100644 100644 4134044... c6887f5... M        src/c/src/addrvec.c
> :100644 100644 5345ebc... 3897095... M        src/c/src/addrvec.h
> :100644 100644 aab214d... 5505427... M        src/c/src/cli.c
> :100644 100644 0d3ae19... 0c513e8... M        src/c/src/load_gen.c
> :100644 100644 21dc70c... eec505c... M        src/c/src/zk_adaptor.h
> :100644 100644 795875e... 93464fb... M        src/c/src/zookeeper.c
> :100644 100644 41d5179... c6536dc... M        src/c/tests/TestClientRetry.cc
> :000000 100644 0000000... 4e22bc7... A        
> src/c/tests/TestReadOnlyClient.cc
> :000000 100644 0000000... 035f26d... A        src/c/tests/WatchUtil.h
> :100644 100644 f97c943... 7de3ed3... M        src/c/tests/ZKMocks.cc
> :100644 100644 fbcfc4f... 2717ded... M        src/c/tests/ZKMocks.h
> :000000 100644 0000000... aa2dd8c... A        src/c/tests/quorum.cfg
> :100755 100755 0af80f8... 2159e20... M        src/c/tests/zkServer.sh
> 
> 
> Diffs
> -----
> 
>   src/c/Makefile.am 6130c4d 
>   src/c/README fe25015 
>   src/c/include/zookeeper.h d5a3876 
>   src/c/src/addrvec.h 5345ebc 
>   src/c/src/addrvec.c 4134044 
>   src/c/src/cli.c aab214d 
>   src/c/src/load_gen.c 0d3ae19 
>   src/c/src/zk_adaptor.h 21dc70c 
>   src/c/src/zookeeper.c 795875e 
>   src/c/tests/TestClientRetry.cc 41d5179 
>   src/c/tests/TestReadOnlyClient.cc PRE-CREATION 
>   src/c/tests/WatchUtil.h PRE-CREATION 
>   src/c/tests/ZKMocks.h fbcfc4f 
>   src/c/tests/ZKMocks.cc f97c943 
>   src/c/tests/quorum.cfg PRE-CREATION 
>   src/c/tests/zkServer.sh 0af80f8 
> 
> Diff: https://reviews.apache.org/r/23081/diff/
> 
> 
> Testing
> -------
> 
> all tests pass, *should* compile on Windows. 
> 
> 
> Thanks,
> 
> Raul Gutierrez Segales
> 
>

Reply via email to