----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38834/#review100976 -----------------------------------------------------------
I really love this JIRA as it will make the code a lot more manageable. Patch needs rebase. I assume you have just moved the code and not changed anything else. Can you please confirm? client/src/main/java/org/apache/falcon/cli/FalconAdminCLI.java (line 37) <https://reviews.apache.org/r/38834/#comment158266> shouldn't version also be here? Is it used elsewhere also and you need it in base class? client/src/main/java/org/apache/falcon/cli/FalconAdminCLI.java (line 38) <https://reviews.apache.org/r/38834/#comment158267> These options were earlier public and I am not sure but they might be used by regression etc. Since they are final, no harm in exposing them I guess. client/src/main/java/org/apache/falcon/cli/FalconAdminCLI.java (line 58) <https://reviews.apache.org/r/38834/#comment158268> Should it be else if instead of if? client/src/main/java/org/apache/falcon/cli/FalconCLI.java (line 155) <https://reviews.apache.org/r/38834/#comment158269> Any reason for not returning the exit value for other commands? client/src/main/java/org/apache/falcon/cli/FalconInstanceCLI.java (line 288) <https://reviews.apache.org/r/38834/#comment158272> Nit: Moving it up at the beginning will improve the readability. - Ajay Yadava On Sept. 29, 2015, 1 a.m., Balu Vellanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38834/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2015, 1 a.m.) > > > Review request for Falcon, Ajay Yadava and Sowmya Ramesh. > > > Bugs: FALCON-592 > https://issues.apache.org/jira/browse/FALCON-592 > > > Repository: falcon-git > > > Description > ------- > > Today, FalconCLI has options for all types of commands, namely instance, > entity, admin, graph and recipe. When I had to write CLI for falcon Metadata, > I separated it out into FalconMetadataCLI. This helped code become more > readable and manageable. > In similar fashion, break up FalconCLI into FalconInstanceCLI, > FalconEntityCLI and so on. > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/cli/FalconAdminCLI.java PRE-CREATION > client/src/main/java/org/apache/falcon/cli/FalconCLI.java c914649 > client/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java > PRE-CREATION > client/src/main/java/org/apache/falcon/cli/FalconInstanceCLI.java > PRE-CREATION > client/src/main/java/org/apache/falcon/cli/FalconMetadataCLI.java dbc553c > client/src/main/java/org/apache/falcon/cli/FalconRecipeCLI.java > PRE-CREATION > client/src/main/java/org/apache/falcon/recipe/RecipeFactory.java 32b0871 > webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java 07aacdd > > Diff: https://reviews.apache.org/r/38834/diff/ > > > Testing > ------- > > Unit and integration tests passed. Tested end2end with simple entity > submission from CLI > > > Thanks, > > Balu Vellanki > >
