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




core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java (line 45)
<https://reviews.apache.org/r/55404/#comment232437>

    You can inline that as never used outside the next line.



core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java (line 46)
<https://reviews.apache.org/r/55404/#comment232438>

    Pls. rename to `HCAT_URI_REGEX_CONFIG`.



core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java (lines 325 - 336)
<https://reviews.apache.org/r/55404/#comment232439>

    Extract to separate `class HCatURIParser`. Should not be `static`.



core/src/main/resources/oozie-default.xml (lines 2967 - 2970)
<https://reviews.apache.org/r/55404/#comment232440>

    `<description />` missing.



core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java (line 560)
<https://reviews.apache.org/r/55404/#comment232450>

    `whenMultipleHCatURIsAreSplitPartsAreExtractedCorrectly()` would be a 
better name.



core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java (lines 561 - 
565)
<https://reviews.apache.org/r/55404/#comment232441>

    Extract method.



core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java (lines 569 - 
572)
<https://reviews.apache.org/r/55404/#comment232442>

    Use extracted method.



core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java (lines 574 - 
577)
<https://reviews.apache.org/r/55404/#comment232443>

    Use extracted method.



sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java (lines 56 - 
62)
<https://reviews.apache.org/r/55404/#comment232444>

    Should be in `class HCatURIParser`.



sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java (lines 64 - 
73)
<https://reviews.apache.org/r/55404/#comment232445>

    Should be in `class HCatURIParser`.



sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java (lines 118 - 
131)
<https://reviews.apache.org/r/55404/#comment232446>

    Should be in `class HCatURIParser`.



sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java (line 49)
<https://reviews.apache.org/r/55404/#comment232449>

    `whenMultipleHCatURIsAreParsedASingleURIIsExtracted()` would be a better 
name.



sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java (lines 
55 - 57)
<https://reviews.apache.org/r/55404/#comment232447>

    JUnit 4's `@Rule public ExpectedException expected = 
ExpectedException.none(); expected.expect(HCatAccessorException.class);` comes 
to the rescue.



sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java (line 56)
<https://reviews.apache.org/r/55404/#comment232448>

    `expected.expectMessage(...)`. Also please do use `Logger` within test 
classes.


- András Piros


On Jan. 11, 2017, 5:41 a.m., Abhishek Bafna wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55404/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2017, 5:41 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2701
>     https://issues.apache.org/jira/browse/OOZIE-2701
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> Oozie to support Multiple HCatalog URIs
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java 9475f72 
>   core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java 67b37ec 
>   core/src/main/resources/oozie-default.xml 943f9bc 
>   core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java e1cf133 
>   sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java 8ec3fae 
>   sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java 
> 1b1a8fa 
> 
> Diff: https://reviews.apache.org/r/55404/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>

Reply via email to