-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40817/#review115636
-----------------------------------------------------------



cli/src/main/java/org/apache/falcon/cli/FalconCLIRuntimeException.java (line 44)
<https://reviews.apache.org/r/40817/#comment176623>

    Just curious, why is it required?



cli/src/main/java/org/apache/falcon/cli/commands/BaseFalconCommands.java (line 
33)
<https://reviews.apache.org/r/40817/#comment176624>

    Hmm.. so our checkstyle allows wildcard imports for static members?



cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 
(line 45)
<https://reviews.apache.org/r/40817/#comment176872>

    required?



cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 
(line 81)
<https://reviews.apache.org/r/40817/#comment176873>

    2 constants with same value, 1 can't be used in all places?



cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 
(line 86)
<https://reviews.apache.org/r/40817/#comment176630>

    If I recall correctly not all of these parameters are mandatory.



cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 
(line 109)
<https://reviews.apache.org/r/40817/#comment176861>

    I think it should have been a String instead of File



cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 
(line 117)
<https://reviews.apache.org/r/40817/#comment176862>

    I believe this method should have been called update



cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 
(line 206)
<https://reviews.apache.org/r/40817/#comment176866>

    nit: should we rename ENTITY_PREFIX_SPACE to ENTITY_COMMAND_PREFIX  or 
ENTITY_PREFIX? Will that be a misrepresentation?



cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 
(line 239)
<https://reviews.apache.org/r/40817/#comment176874>

    why is ENTITY_PREFIX required for validating orderBy?



cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 
(line 248)
<https://reviews.apache.org/r/40817/#comment176870>

    I think we should be able to directly write it as final EntityType type 
instead of final String entityType.
    
    If yes, then please make that change for all commands.



cli/src/test/java/org/apache/falcon/cli/commands/FalconConnectionCommandsTest.java
 (line 47)
<https://reviews.apache.org/r/40817/#comment176871>

    no tests for Entity Commands?



client/src/main/java/org/apache/falcon/resource/EntityList.java (line 55)
<https://reviews.apache.org/r/40817/#comment176875>

    This might affect other consumers of these fields. All of them might not 
show during compile time e.g. in regression code base they might be depending 
on these being static.


- Ajay Yadava


On Jan. 11, 2016, 11:32 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40817/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2016, 11:32 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1609
>     https://issues.apache.org/jira/browse/FALCON-1609
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> putting for early review
> 
> 
> Diffs
> -----
> 
>   cli/src/main/java/org/apache/falcon/cli/FalconCLIRuntimeException.java 
> b7fa4cd524bfcd50ac260dadc2a1f94021b3df03 
>   cli/src/main/java/org/apache/falcon/cli/commands/BaseFalconCommands.java 
> dbd28fb74e2aaeedc94ed4ee43e688ea9efb15b0 
>   
> cli/src/main/java/org/apache/falcon/cli/commands/FalconConnectionCommands.java
>  cabe5a8083daf90456636d3378e410e3d95a4ada 
>   cli/src/main/java/org/apache/falcon/cli/commands/FalconEntityCommands.java 
> 6e091efcbe26ff7df7a3fe52e33b20f781b8d785 
>   
> cli/src/main/java/org/apache/falcon/cli/commands/FalconInstanceCommands.java 
> 8f3a2fc3ef57bb9026b98c85f51b363234d607c6 
>   cli/src/test/java/org/apache/falcon/cli/commands/FalconCLITest.java 
> PRE-CREATION 
>   
> cli/src/test/java/org/apache/falcon/cli/commands/FalconConnectionCommandsTest.java
>  PRE-CREATION 
>   client/src/main/java/org/apache/falcon/cli/FalconCLI.java 
> 24f230ae66ba710945be76d176fbdbdbbcb3721c 
>   client/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java 
> cb39a2fe95beae32dee930afb9bb78d85ebb58ea 
>   client/src/main/java/org/apache/falcon/client/FalconCLIException.java 
> 51ef952474fda1242f4b4b5a06ccfd3011ea533a 
>   client/src/main/java/org/apache/falcon/client/FalconClient.java 
> b4b176224554fb5a86ebca2bcced980dc84d2dd6 
>   client/src/main/java/org/apache/falcon/resource/EntityList.java 
> f58169140af0f633463be93cff607c32e22254f4 
> 
> Diff: https://reviews.apache.org/r/40817/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>

Reply via email to