> On Sept. 29, 2015, 5:46 p.m., Ajay Yadava wrote: > > client/src/main/java/org/apache/falcon/cli/FalconAdminCLI.java, line 37 > > <https://reviews.apache.org/r/38834/diff/1/?file=1086309#file1086309line37> > > > > shouldn't version also be here? Is it used elsewhere also and you need > > it in base class?
Version is used in baseclass. So I need to keep it in FalconCLI > On Sept. 29, 2015, 5:46 p.m., Ajay Yadava wrote: > > client/src/main/java/org/apache/falcon/cli/FalconAdminCLI.java, line 38 > > <https://reviews.apache.org/r/38834/diff/1/?file=1086309#file1086309line38> > > > > 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. I looked at Falcon regression and confirmed that they are not being used in Falcon regression code. > On Sept. 29, 2015, 5:46 p.m., Ajay Yadava wrote: > > client/src/main/java/org/apache/falcon/cli/FalconAdminCLI.java, line 58 > > <https://reviews.apache.org/r/38834/diff/1/?file=1086309#file1086309line58> > > > > Should it be else if instead of if? STACK_OPTION can be used along with other options. So it should not be "else if" > On Sept. 29, 2015, 5:46 p.m., Ajay Yadava wrote: > > client/src/main/java/org/apache/falcon/cli/FalconCLI.java, line 218 > > <https://reviews.apache.org/r/38834/diff/1/?file=1086310#file1086310line218> > > > > Any reason for not returning the exit value for other commands? entityCommand, instanceCommand, metadataCommand and recipeCommand return void. Hence we are not returning the exit value. > On Sept. 29, 2015, 5:46 p.m., Ajay Yadava wrote: > > client/src/main/java/org/apache/falcon/cli/FalconInstanceCLI.java, line 288 > > <https://reviews.apache.org/r/38834/diff/1/?file=1086312#file1086312line288> > > > > Nit: Moving it up at the beginning will improve the readability. Agreed, made all changes in Falcon*CLI - Balu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38834/#review100976 ----------------------------------------------------------- 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 > >
