> On 2011-12-14 23:36:06, Patrick Hunt wrote: > > /src/java/main/org/apache/zookeeper/jmx/MBeanRegistry.java, lines 126-127 > > <https://reviews.apache.org/r/2208/diff/1/?file=48123#file48123line126> > > > > cleanup this typo (2 returns)
Done. > On 2011-12-14 23:36:06, Patrick Hunt wrote: > > /src/java/main/org/apache/zookeeper/jmx/MBeanRegistry.java, line 130 > > <https://reviews.apache.org/r/2208/diff/1/?file=48123#file48123line130> > > > > let's call this > > > > getRegisteredBeans() > > Ok. > On 2011-12-14 23:36:06, Patrick Hunt wrote: > > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, lines > > 757-767 > > <https://reviews.apache.org/r/2208/diff/1/?file=48124#file48124line757> > > > > Add back the try/catch protection. This was necessary in some cases (I > > can't remember why off the top of my head though). I've enhanced the exception handling during "public void unregister(ZKMBeanInfo bean)" method invocation since JMException was caught and logged two times: 1) private void unregister(String path,ZKMBeanInfo bean) throws JMException, 2) public void unregister(ZKMBeanInfo bean). I've also added a "catch(Throwable t)" to "public void unregister(ZKMBeanInfo bean)" method that will generate an error log instead of a warn. > On 2011-12-14 23:36:06, Patrick Hunt wrote: > > /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java, line 1 > > <https://reviews.apache.org/r/2208/diff/1/?file=48126#file48126line1> > > > > license header is missing. add the std boilerplate found on any other > > java file in the project. As you can see I'm a newbie contributor ;-) > On 2011-12-14 23:36:06, Patrick Hunt wrote: > > /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java, line 15 > > <https://reviews.apache.org/r/2208/diff/1/?file=48126#file48126line15> > > > > add a comment to the class describing what this set of tests is for Done. > On 2011-12-14 23:36:06, Patrick Hunt wrote: > > /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java, line 16 > > <https://reviews.apache.org/r/2208/diff/1/?file=48126#file48126line16> > > > > no tabs, reformat with spaces only. (you'll probably need to cleanup > > some of your other changes at the same time) Done. It would be nice that the eclipse ant task will create the .setting files to set all formatting rules and useful save actions (like apply formatting). I'll investigate it and upload as a new jira if success. > On 2011-12-14 23:36:06, Patrick Hunt wrote: > > /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java, line 19 > > <https://reviews.apache.org/r/2208/diff/1/?file=48126#file48126line19> > > > > it's fine to ref a jira here, however there should also be some details > > on what this test is doing. (short summary esp as the test itself is pretty > > clear) Done. - César ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2208/#review3914 ----------------------------------------------------------- On 2011-10-05 11:59:30, César Álvarez Núñez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2208/ > ----------------------------------------------------------- > > (Updated 2011-10-05 11:59:30) > > > Review request for zookeeper. > > > Summary > ------- > > See https://issues.apache.org/jira/browse/ZOOKEEPER-1214 > > > This addresses bug ZOOKEEPER-1214. > https://issues.apache.org/jira/browse/ZOOKEEPER-1214 > > > Diffs > ----- > > /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1179165 > /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java PRE-CREATION > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1179166 > /src/java/main/org/apache/zookeeper/jmx/MBeanRegistry.java 1169669 > > Diff: https://reviews.apache.org/r/2208/diff > > > Testing > ------- > > - > > > Thanks, > > César > >
