Also, not sure how this will be staged in with https://issues.apache.org/jira/browse/ZOOKEEPER-1344 that adds chroot support to the multiop. But might want to look at those changes to see how they affect these changes...
On Fri, Mar 9, 2012 at 8:23 PM, Marshall McMullen < [email protected]> wrote: > I completely agree with Ted on the need for unit tests. Everything looks > correct, but tests would make me feel much more confident. > > On Fri, Mar 9, 2012 at 12:02 PM, Ted Dunning <[email protected]> wrote: > >> >> ----------------------------------------------------------- >> This is an automatically generated e-mail. To reply, visit: >> https://reviews.apache.org/r/4264/#review5795 >> ----------------------------------------------------------- >> >> >> This still needs unit tests. Reviewing in this form will only tell us if >> the changes seem correct, not whether all the required changes have been >> made. >> >> Until there are tests, it does much matter to review this any further. >> >> - Ted >> >> >> On 2012-03-09 05:43:21, Ted Yu wrote: >> > >> > ----------------------------------------------------------- >> > This is an automatically generated e-mail. To reply, visit: >> > https://reviews.apache.org/r/4264/ >> > ----------------------------------------------------------- >> > >> > (Updated 2012-03-09 05:43:21) >> > >> > >> > Review request for zookeeper. >> > >> > >> > Summary >> > ------- >> > >> > There is use case where GetData and GetChildren would participate in >> Multi. >> > >> > >> > Diffs >> > ----- >> > >> > /src/java/main/org/apache/zookeeper/Op.java 1298626 >> > /src/java/main/org/apache/zookeeper/server/DataTree.java 1298626 >> > /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java >> 1298626 >> > /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java >> 1298626 >> > /src/zookeeper.jute 1298626 >> > >> > Diff: https://reviews.apache.org/r/4264/diff >> > >> > >> > Testing >> > ------- >> > >> > >> > Thanks, >> > >> > Ted >> > >> > >> >> >
