> On Dec. 16, 2014, 11:24 a.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java, > > line 159 > > <https://reviews.apache.org/r/28835/diff/2/?file=792808#file792808line159> > > > > There's no need to have a separate method just to call another method. > > I would call progressable directly above.
ok > On Dec. 16, 2014, 11:24 a.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java, > > line 1277 > > <https://reviews.apache.org/r/28835/diff/1-2/?file=786288#file786288line1277> > > > > trailing whitespace ok > On Dec. 16, 2014, 11:24 a.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java, > > line 84 > > <https://reviews.apache.org/r/28835/diff/2/?file=792808#file792808line84> > > > > Use guava threadfactorybuilder. Also, make the created thread a daemon > > thread, so that even if it does get leaked, it doesn't keep the process > > alive. ok > On Dec. 16, 2014, 11:24 a.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java, > > line 146 > > <https://reviews.apache.org/r/28835/diff/2/?file=792808#file792808line146> > > > > Need to reset interruptable status on thread. For the other exceptions, > > you can use the > > catch (IOException|ExecutableException e) > > syntax, as we compile to java7 now ok > On Dec. 16, 2014, 11:24 a.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java, > > line 162 > > <https://reviews.apache.org/r/28835/diff/2/?file=792808#file792808line162> > > > > replace > > }); > > with > > }).get(); > > > > Then the latch is completely unneeded. ok - Rakesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28835/#review65188 ----------------------------------------------------------- On Dec. 16, 2014, 4:07 a.m., Rakesh R wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28835/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2014, 4:07 a.m.) > > > Review request for bookkeeper, fpj, Ivan Kelly, and Sijie Guo. > > > Bugs: BOKKEEPER-634 > https://issues.apache.org/jira/browse/BOKKEEPER-634 > > > Repository: bookkeeper-git > > > Description > ------- > > admin tool for changing the bookie's identifier to IP/hostname > > > Diffs > ----- > > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java > a1e4639 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java > PRE-CREATION > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerOpTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/28835/diff/ > > > Testing > ------- > > Added few tests > > > Thanks, > > Rakesh R > >
