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



lens-cli/src/main/java/org/apache/lens/cli/commands/BaseLensCommand.java (line 
146)
<https://reviews.apache.org/r/36178/#comment143492>

    Can you make sure all possible scenarios are covered by checking usages of 
this function? Wherever this function is used, we have a possible place where 
String can be changed to File. 
    
    Also can you verify commands that expect directory path work fine? e.g. 
query results.



lens-cli/src/main/java/org/apache/lens/cli/commands/BaseLensCommand.java (line 
148)
<https://reviews.apache.org/r/36178/#comment143493>

    Can we avoid explicit check by using `@NonNull` either here of in the 
command arguments where `File` object is argument.



lens-cli/src/main/java/org/apache/lens/cli/commands/LensConnectionCommands.java 
(line 101)
<https://reviews.apache.org/r/36178/#comment143494>

    I'm not sure, but shouldn't we do `path.getPath` here? Can you verify by 
passing both default path -- absolute and relative -- and fully qualified path?
    
    1. add jar local/path
    2. add jar /full/path
    3. add jar file:///full/path
    4. add jar file://local/path
    
    Can probably add these to test cases.



lens-cli/src/main/java/org/apache/lens/cli/commands/LensCubeCommands.java (line 
60)
<https://reviews.apache.org/r/36178/#comment143495>

    `create` can probably take a `File` object now. This can propagate all the 
way upto `LensMetadataClient`. Similarly for other functions. 
    
    I'm just suggesting based on my understanding of the modules. If making 
this change might break something, we can leave it. But it'll be good to have 
an understanding of why we can or can't make such a change.


- Rajat Khandelwal


On July 5, 2015, 12:18 a.m., Yash Sharma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36178/
> -----------------------------------------------------------
> 
> (Updated July 5, 2015, 12:18 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Added changes for file name suggestions on Tab. Changed String file paths to 
> File.
> 
> 
> https://issues.apache.org/jira/browse/LENS-634
> 
> 
> Diffs
> -----
> 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/BaseLensCommand.java 
> 74b4e91 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/LensConnectionCommands.java
>  8b4face 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensCubeCommands.java 
> 6ba702f 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/LensDimensionCommands.java
>  de022c1 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/LensDimensionTableCommands.java
>  6a93393 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensFactCommands.java 
> bdb9c38 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensQueryCommands.java 
> c48fabd 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/LensStorageCommands.java 
> 928120a 
>   
> lens-cli/src/test/java/org/apache/lens/cli/TestLensConnectionCliCommands.java 
> 7503221 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensCubeCommands.java 
> 73661e1 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensDatabaseCommands.java 
> b6f96e6 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensDimensionCommands.java 
> a22862e 
>   
> lens-cli/src/test/java/org/apache/lens/cli/TestLensDimensionTableCommands.java
>  a87d0b7 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommands.java 
> 13bfbd2 
>   
> lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommandsWithMissingWeight.java
>  73f3a78 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java 
> 46e28a9 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensStorageCommands.java 
> 3a04ec6 
> 
> Diff: https://reviews.apache.org/r/36178/diff/
> 
> 
> Testing
> -------
> 
> New build results-
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules .............................. SUCCESS [  2.460 
> s]
> [INFO] Lens ............................................... SUCCESS [  2.842 
> s]
> [INFO] Lens API ........................................... SUCCESS [ 21.035 
> s]
> [INFO] Lens API for server and extensions ................. SUCCESS [ 21.545 
> s]
> [INFO] Lens Cube .......................................... SUCCESS [03:04 
> min]
> [INFO] Lens DB storage .................................... SUCCESS [ 19.023 
> s]
> [INFO] Lens Query Library ................................. SUCCESS [ 14.263 
> s]
> [INFO] Lens Hive Driver ................................... SUCCESS [02:28 
> min]
> [INFO] Lens Driver for JDBC ............................... SUCCESS [ 34.581 
> s]
> [INFO] Lens Server ........................................ SUCCESS [05:10 
> min]
> [INFO] Lens client ........................................ SUCCESS [ 36.137 
> s]
> [INFO] Lens CLI ........................................... SUCCESS [02:06 
> min]
> [INFO] Lens Examples ...................................... SUCCESS [  9.193 
> s]
> [INFO] Lens Distribution .................................. SUCCESS [  7.860 
> s]
> [INFO] Lens ML Lib ........................................ SUCCESS [01:19 
> min]
> [INFO] Lens ML Ext Distribution ........................... SUCCESS [  3.114 
> s]
> [INFO] Lens Regression .................................... SUCCESS [ 12.437 
> s]
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 17:15 min
> [INFO] Finished at: 2015-07-05T00:12:45+05:30
> [INFO] Final Memory: 125M/375M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Yash Sharma
> 
>

Reply via email to