No further suggestions, not even naming variables. Great job, congrats! I concur with Alex: ship it!
Edward Em 21/08/2013 19:11, "Helen Hastings" <[email protected]> escreveu: > > > > On Aug. 20, 2013, 2:31 a.m., Edward Ribeiro wrote: > > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java, > line 402 > > > < > https://reviews.apache.org/r/12983/diff/3/?file=342272#file342272line402> > > > > > > Why did you renamed quorumVerifier to qv? It's a good practice to > let the full name as opposed to initials (qv, obs, fol, etc), at least in > Java. I didn't get the raison d'etre of this change... > > "quorumVerifier" is the name of the protected instance variable of type > QuorumVerifier belonging to the QuorumPeerConfig class. Previously, the > method was altering this variable. Now, it is returning a new > QuorumVerifier object. I don't think it would make sense to give a local > variable the same name as an instance variable. "qv" is used by many other > ZooKeeper methods so I decided to follow suit. Any other suggestions for > naming? I will change "obs" and "fol" to full names in the test. > > > - Helen > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12983/#review25337 > ----------------------------------------------------------- > > > On Aug. 21, 2013, 10:11 p.m., Helen Hastings wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/12983/ > > ----------------------------------------------------------- > > > > (Updated Aug. 21, 2013, 10:11 p.m.) > > > > > > Review request for zookeeper and Alexander Shraer. > > > > > > Repository: zookeeper > > > > > > Description > > ------- > > > > Added standaloneEnabled flag to QuorumPeerConfig to give the option to > disable standalone mode for a single server. > > > > > > Diffs > > ----- > > > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/StandaloneDisabledTest.javaPRE-CREATION > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ReconfigTest.java1516310 > > > http://svn.apache.org/repos/asf/zookeeper/trunk/src/java/test/org/apache/zookeeper/test/StandaloneTest.java1516310 > > > > Diff: https://reviews.apache.org/r/12983/diff/ > > > > > > Testing > > ------- > > > > > > Thanks, > > > > Helen Hastings > > > > > >
