> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
> > Lines 152 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067411#file2067411line152>
> >
> >     Better naming suggested: ActionConfVerifier

Done.


> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067428#file2067428line38>
> >
> >     General thoughts: this class has quite a few protected methods. We have 
> > to think about whether it's necessary or not. If the class is not going to 
> > be subclassed, making them "private" is preferable.

Done.


> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 43 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067428#file2067428line43>
> >
> >     I'd prefer this as being private, with having a package private setter 
> > method.

Done.


> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
> > Lines 125 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067429#file2067429line125>
> >
> >     This string can be placed directly in the constructor.

Done.


> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/GitServer.java
> > Lines 74 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067430#file2067430line74>
> >
> >     At least a warning/error message would be good here.

Done.


- András


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


On Aug. 3, 2018, 10 p.m., Clay B. wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
> 
> (Updated Aug. 3, 2018, 10 p.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Bugs: OOZIE-2877
>     https://issues.apache.org/jira/browse/OOZIE-2877
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> OOZIE-2877 - Oozie Git Action
> 
> 
> Diffs
> -----
> 
>   client/src/main/resources/git-action-1.0.xsd PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml b69d2c9 
>   core/src/test/java/org/apache/oozie/test/XTestCase.java 661970d 
>   docs/src/site/twiki/WorkflowFunctionalSpec.twiki 76cbe21 
>   examples/src/main/apps/git/job.properties PRE-CREATION 
>   examples/src/main/apps/git/workflow.xml PRE-CREATION 
>   fluent-job/fluent-job-api/pom.xml 4c9b853 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java
>  PRE-CREATION 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java
>  PRE-CREATION 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java
>  7bb82e5 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java
>  PRE-CREATION 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java
>  PRE-CREATION 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java
>  ec56554 
>   fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e 
>   fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890 
>   pom.xml 92358aa 
>   sharelib/git/pom.xml PRE-CREATION 
>   sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java 
> PRE-CREATION 
>   
> sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java 
> PRE-CREATION 
>   sharelib/git/src/test/java/org/apache/oozie/action/hadoop/GitServer.java 
> PRE-CREATION 
>   
> sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
>  PRE-CREATION 
>   sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java 
> PRE-CREATION 
>   
> sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
>  PRE-CREATION 
>   sharelib/pom.xml 6a0864d 
>   src/main/assemblies/sharelib.xml 07dc69c 
>   webapp/pom.xml fd3f89f 
> 
> 
> Diff: https://reviews.apache.org/r/59620/diff/10/
> 
> 
> Testing
> -------
> 
> Tested using unit and integration tests. Still need to:
> * Test on a cluster
> * Test with an authenticated SSH hosted Git repo
> 
> Sumitted a request to the JGit community as their branch pulling code seems 
> to have an 
> [issue](https://dev.eclipse.org/mhonarc/lists/jgit-dev/msg03343.html).
> 
> Andras has tested against his patch 011 with results at 
> https://issues.apache.org/jira/browse/OOZIE-2877?focusedCommentId=16459523&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16459523
> 
> 
> File Attachments
> ----------------
> 
> 0001-OOZIE-2877-Oozie-Git-Action.patch
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/29/24f90a78-3dc1-49fe-bf29-5927a3cd5e72__0001-OOZIE-2877-Oozie-Git-Action.patch
> Patch
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/29/dd23dd72-67e0-456f-9b52-e566d8d17d16__0001-OOZIE-2877-Oozie-Git-Action.patch
> 
> 
> Thanks,
> 
> Clay B.
> 
>

Reply via email to