[ https://issues.apache.org/jira/browse/ZOOKEEPER-729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12855251#action_12855251 ]
Henry Robinson commented on ZOOKEEPER-729: ------------------------------------------ Patch looks pretty good! A few nits: 1. Your tab level seems to be set to two spaces, but the standard is to use four tabs per space 2. I would change the LOG levels to DEBUG instead of INFO. 3. Consider calling listSubTree listSubTreeBFS? I agree about making the strategy explicit, I feel this is the best way to do it. 4. You should be explicit in the listSubTree comment that this is *not* an atomic snapshot and does not necessarily represent the state of the sub-tree as it ever was - particularly given that this is a public method. 5. Similarly, explicitly note in the comments that you are deleting all nodes regardless of their version. 6. Did you decide against including a -r option for delete? I'm +1 on that idea rather than rmr, but don't feel that strongly. 7. I would remove the TODO comments - I'm not a huge fan of them (anymore :) ). There is no separator constant, the BFS thing should be taken care of by renaming and we must make a decision about dealing with the async requests - my take is just to do what you are doing and just run through the entire tree. Finally - we need a test for this! I know there's not a lot of coverage of ZooKeeper, which is not your fault - but I'm intending to be fairly militant about tests :) Check out e.g. ACLRootTest for a structure to get a server up and running and a client to connect to it. A couple of tests which verify the behaviour of both variants would be great. Ideally, we'd be able to test the behaviour when a create interleaves with a recursive delete, but that might still be hard to do without mocking up some of the code. I'll be happy to commit this once these are sorted out, thanks for your patience. > Recursively delete a znode - zkCli.sh rmr /node > ------------------------------------------------ > > Key: ZOOKEEPER-729 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-729 > Project: Zookeeper > Issue Type: New Feature > Components: java client > Reporter: Kay Kay > Assignee: Kay Kay > Fix For: 3.4.0 > > Attachments: ZOOKEEPER-729.patch, ZOOKEEPER-729.patch > > > Recursively delete a given znode in zookeeper, from the command-line. > New operation "rmr" added to zkclient. > $ ./zkCli.sh rmr /node -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.