falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ajay Yadava" <ajayn...@gmail.com>
Subject Re: Review Request 38834: Refactor FalconCLI to make it more manageable.
Date Tue, 29 Sep 2015 17:46:32 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message