----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69594/#review211473 -----------------------------------------------------------
Nice job so far. I really would rather extract functionality into a nested class inside `PurgeXCommand`, and test it separately, for better readability and SRP. core/src/main/java/org/apache/oozie/command/PurgeXCommand.java Lines 212-213 (original), 214-232 (patched) <https://reviews.apache.org/r/69594/#comment296647> I'm wondering whether it covers an arbitrarily deep `WorkflowJobBean` -> `WorkflowActionBean` -> `WorkflowJobBean` structure. If so, are we checking for DAGness anywhere? core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Lines 2871 (patched) <https://reviews.apache.org/r/69594/#comment296648> Please give a more descriptive test method name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Lines 2898 (patched) <https://reviews.apache.org/r/69594/#comment296650> Please give a more descriptive test method name. core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java Lines 2925 (patched) <https://reviews.apache.org/r/69594/#comment296651> Please give a more descriptive test method name. - András Piros On Dec. 19, 2018, 9:57 a.m., Andras Salamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69594/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2018, 9:57 a.m.) > > > Review request for oozie, András Piros and Kinga Marton. > > > Repository: oozie-git > > > Description > ------- > > OOZIE-3400: Fix PurgeService sub-sub-workflow checking > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/command/PurgeXCommand.java 42c3b28a6 > core/src/test/java/org/apache/oozie/command/TestPurgeXCommand.java > d11fcffbb > > > Diff: https://reviews.apache.org/r/69594/diff/1/ > > > Testing > ------- > > Run TestPurgeXCommand unit tests locally. > > > Thanks, > > Andras Salamon > >
