> On July 27, 2015, 9:12 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/util/ApiUtilityMethods.java, 
> > line 68
> > <https://reviews.apache.org/r/36178/diff/6-7/?file=1018821#file1018821line68>
> >
> >     Why this change? Regular expression seems fishy to me. Can you explain? 
> > Won't `/+` work too? And why are we replacing `//////` with `/`?

Yes /+ should work. The only purpose is to replace all multiple slashes with a 
single slash. This is the way the CLI sends the paths. Also this will let us 
handle anypath that has extra slashes erronously like hdfs://// or s3://///

Will update the patch and post back.


> On July 27, 2015, 9:12 a.m., Rajat Khandelwal wrote:
> > lens-api/src/test/java/org/apache/lens/api/TestApiUtilityMethods.java, 
> > lines 73-87
> > <https://reviews.apache.org/r/36178/diff/7/?file=1021880#file1021880line73>
> >
> >     Separate tests can be separate methods. Thus making them modular. 
> >     
> >     If a test will throw exception. Please use `@Test(expectedExceptions = 
> > ...)` instead of catch and assert.

Cool. Will implement. Thanks.


- Yash


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


On July 25, 2015, 1:08 p.m., Yash Sharma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36178/
> -----------------------------------------------------------
> 
> (Updated July 25, 2015, 1:08 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-634
>     https://issues.apache.org/jira/browse/LENS-634
> 
> 
> 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-api/src/main/java/org/apache/lens/api/util/ApiUtilityMethods.java 
> PRE-CREATION 
>   lens-api/src/test/java/org/apache/lens/api/TestApiUtilityMethods.java 
> PRE-CREATION 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/BaseLensCommand.java 
> bdfae24 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensCRUDCommand.java 
> 9cb7d69 
>   
> 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
>  5fd80a1 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensFactCommands.java 
> 5493dba 
>   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/main/java/org/apache/lens/cli/commands/PhysicalTableCrudCommand.java
>  c505680 
>   
> 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
>  56e962d 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommands.java 
> fa342c8 
>   
> lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommandsWithMissingWeight.java
>  73f3a78 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java 
> 319077a 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensStorageCommands.java 
> 3a04ec6 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  5e0f24a 
>   lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java 
> 92687c3 
>   src/site/apt/user/cli.apt 63f2b3f 
> 
> Diff: https://reviews.apache.org/r/36178/diff/
> 
> 
> Testing
> -------
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules .............................. SUCCESS [  2.260 
> s]
> [INFO] Lens ............................................... SUCCESS [  2.762 
> s]
> [INFO] Lens API ........................................... SUCCESS [ 23.013 
> s]
> [INFO] Lens API for server and extensions ................. SUCCESS [ 20.134 
> s]
> [INFO] Lens Cube .......................................... SUCCESS [03:53 
> min]
> [INFO] Lens DB storage .................................... SUCCESS [ 19.727 
> s]
> [INFO] Lens Query Library ................................. SUCCESS [ 14.522 
> s]
> [INFO] Lens Hive Driver ................................... SUCCESS [02:30 
> min]
> [INFO] Lens Driver for JDBC ............................... SUCCESS [ 33.807 
> s]
> [INFO] Lens Server ........................................ SUCCESS [05:00 
> min]
> [INFO] Lens client ........................................ SUCCESS [ 37.785 
> s]
> [INFO] Lens CLI ........................................... SUCCESS [02:04 
> min]
> [INFO] Lens Examples ...................................... SUCCESS [ 10.077 
> s]
> [INFO] Lens Distribution .................................. SUCCESS [  7.065 
> s]
> [INFO] Lens ML Lib ........................................ SUCCESS [01:21 
> min]
> [INFO] Lens ML Ext Distribution ........................... SUCCESS [  2.631 
> s]
> [INFO] Lens Regression .................................... SUCCESS [ 11.392 
> s]
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 17:57 min
> [INFO] Finished at: 2015-07-25T18:33:51+05:30
> [INFO] Final Memory: 130M/389M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Yash Sharma
> 
>

Reply via email to