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



bin/zkServer.sh
<https://reviews.apache.org/r/24786/#comment88713>

    consider merging the two if statements by deleting lines 188, 189 and 190



src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
<https://reviews.apache.org/r/24786/#comment88712>

    isFile is probably better than exists



src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
<https://reviews.apache.org/r/24786/#comment88714>

    instead of lines 593 and 594 perhaps do:
    
    if (isDistributed()) {...



src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
<https://reviews.apache.org/r/24786/#comment88711>

    by now you already parsed this file if it exists, so you can check serverId 
instead of checking the file:
    
    if (serverId == UNSET_SERVERID)...



src/java/test/org/apache/zookeeper/test/StandaloneTest.java
<https://reviews.apache.org/r/24786/#comment88715>

    we should probably keep the comment about HBase:
    
    This handles the case of HBase, which configures zoo.cfg in this way. 
Maintain b/w compatibility.


- Alexander Shraer


On Aug. 17, 2014, 9:58 p.m., Hongchao Deng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24786/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2014, 9:58 p.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Repository: zookeeper-git
> 
> 
> Description
> -------
> 
> ZOOKEEPER-2006
> 
> 
> Diffs
> -----
> 
>   bin/zkServer.sh 74d9dc2 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 
> 02edcf0 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 
> cb2b1c3 
>   src/java/test/org/apache/zookeeper/test/StandaloneTest.java 1701ba3 
> 
> Diff: https://reviews.apache.org/r/24786/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hongchao Deng
> 
>

Reply via email to