----------------------------------------------------------- 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 > >
