You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Hongchao Deng <hd...@cloudera.com> on 2014/07/30 01:01:05 UTC

Review Request 24074: ZOOKEEPER-1993

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

Review request for zookeeper.


Repository: zookeeper-git


Description
-------

ZOOKEEPER-1993


Diffs
-----

  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java a01df53 
  src/java/test/org/apache/zookeeper/server/quorum/DynamicConfigBackwardCompatibilityTest.java bd4f043 
  src/java/test/org/apache/zookeeper/test/StandaloneTest.java 1701ba3 

Diff: https://reviews.apache.org/r/24074/diff/


Testing
-------


Thanks,

Hongchao Deng

Re: Review Request 24074: ZOOKEEPER-1993

Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24074/#review49124
-----------------------------------------------------------



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

    nit but readability matters: it's much simpler like this:
    
    ```
    if (standaloneEnable) {
       return;
    }
    
    // everything else



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

    again, you can do with less indentation:
    
    if (!nextDynamicConfigFile.exists()) {
       return;
    }
    
    // the rest 



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

    these lines, for reading Properties, are duplicated on line 257. Can we have a readProperties() method to which you pass a file path and get back a populated Properties object?



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

    parentheseses to clarify logical groups please.


- Raul Gutierrez Segales


On July 29, 2014, 11:46 p.m., Hongchao Deng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24074/
> -----------------------------------------------------------
> 
> (Updated July 29, 2014, 11:46 p.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Repository: zookeeper-git
> 
> 
> Description
> -------
> 
> ZOOKEEPER-1993
> 
> 
> Diffs
> -----
> 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java a01df53 
>   src/java/test/org/apache/zookeeper/server/quorum/DynamicConfigBackwardCompatibilityTest.java bd4f043 
>   src/java/test/org/apache/zookeeper/test/StandaloneTest.java 1701ba3 
> 
> Diff: https://reviews.apache.org/r/24074/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hongchao Deng
> 
>

Re: Review Request 24074: ZOOKEEPER-1993

Posted by Hongchao Deng <hd...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24074/
-----------------------------------------------------------

(Updated July 29, 2014, 11:46 p.m.)


Review request for zookeeper.


Changes
-------

Remove trailing space.
Add back the duplicate server list check


Repository: zookeeper-git


Description
-------

ZOOKEEPER-1993


Diffs (updated)
-----

  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java a01df53 
  src/java/test/org/apache/zookeeper/server/quorum/DynamicConfigBackwardCompatibilityTest.java bd4f043 
  src/java/test/org/apache/zookeeper/test/StandaloneTest.java 1701ba3 

Diff: https://reviews.apache.org/r/24074/diff/


Testing
-------


Thanks,

Hongchao Deng