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




core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 97-98 (patched)
<https://reviews.apache.org/r/68505/#comment291657>

    Why JA002 and not GIT002?



docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Lines 1609 (patched)
<https://reviews.apache.org/r/68505/#comment291658>

    typo: aN absolute path



examples/src/main/apps/git/workflow.xml
Lines 24 (patched)
<https://reviews.apache.org/r/68505/#comment291659>

    Is it a good idea to refer to this specific github directory? What happens 
if they delete it? Why not refer to apache/oozie instead?



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 58-59 (patched)
<https://reviews.apache.org/r/68505/#comment291666>

    Why mixing System.out and outAndLog?



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 77-79 (patched)
<https://reviews.apache.org/r/68505/#comment291664>

    What is the purpose of this method? Contrary to the name it only calls 
System.out.println.



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 81-83 (patched)
<https://reviews.apache.org/r/68505/#comment291665>

    What is the purpose of this method? Contrary to the name it only calls 
System.err.println.



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 145 (patched)
<https://reviews.apache.org/r/68505/#comment291669>

    tempDir would be a better name



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
Lines 126 (patched)
<https://reviews.apache.org/r/68505/#comment291670>

    typo: cloneing vs cloning



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
Lines 128 (patched)
<https://reviews.apache.org/r/68505/#comment291672>

    please rename to tempDir



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
Lines 138 (patched)
<https://reviews.apache.org/r/68505/#comment291673>

    formatting: missing spaces around :



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/GitServer.java
Lines 104 (patched)
<https://reviews.apache.org/r/68505/#comment291675>

    formatting: line break before catch



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/GitServer.java
Lines 133 (patched)
<https://reviews.apache.org/r/68505/#comment291676>

    formatting: line break before catch



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/GitServer.java
Lines 142 (patched)
<https://reviews.apache.org/r/68505/#comment291674>

    trailing space



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/GitServer.java
Lines 147 (patched)
<https://reviews.apache.org/r/68505/#comment291677>

    formatting: line break before catch



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 84 (patched)
<https://reviews.apache.org/r/68505/#comment291680>

    this comment is not too useful. Either delete it or explain the reason the 
specify this specific permission.



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 114 (patched)
<https://reviews.apache.org/r/68505/#comment291679>

    formatting: line break before catch



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 92-93 (patched)
<https://reviews.apache.org/r/68505/#comment291683>

    assertTrue("...", WorkflowJob.Status.KILLED, 
client.getJobInfo(jobId).getStatus())



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 106 (patched)
<https://reviews.apache.org/r/68505/#comment291684>

    why test a constant String length?



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 64 (patched)
<https://reviews.apache.org/r/68505/#comment291685>

    formatting: new line before finally



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 115 (patched)
<https://reviews.apache.org/r/68505/#comment291686>

    externalId would be a better name



sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 116 (patched)
<https://reviews.apache.org/r/68505/#comment291687>

    trackerUri would be a better name


- Andras Salamon


On Aug. 24, 2018, 6:25 p.m., András Piros wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68505/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2018, 6:25 p.m.)
> 
> 
> Review request for oozie, Andras Salamon, Clay B., Peter Cseh, Kinga Marton, 
> and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> OOZIE-2877 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 
> b69d2c9aacc1a7f5f903fb80f77bbf5482feb3b5 
>   core/src/test/java/org/apache/oozie/test/XTestCase.java 
> 661970d57ac038d7311856a93f5221f081e9ce15 
>   docs/src/site/twiki/WorkflowFunctionalSpec.twiki 
> 76cbe21ecd79eae86885a8db77831bf2c40f2d4f 
>   examples/src/main/apps/git/job.properties PRE-CREATION 
>   examples/src/main/apps/git/workflow.xml PRE-CREATION 
>   examples/src/main/java/org/apache/oozie/example/fluentjob/Git.java 
> PRE-CREATION 
>   fluent-job/fluent-job-api/pom.xml cefd43c143550a24ffba5a1d6922b8241dd5756a 
>   
> 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/DistcpConfigurationConverter.java
>  fffb734e7c1b920e94f76e6a2b3062dd5aa73a86 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java
>  7bb82e5cbbffbee02785361eb2bbde2fc1c31e7a 
>   
> 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/mapping/Hive2ConfigurationConverter.java
>  c67b5aeab41e0c2d963c0ad3cf44fd19fe7c66e3 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/HiveConfigurationConverter.java
>  5f9a2b180623b89c793e8e6bb6e03d445c0ad991 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/InlineWorkflowConfigurationConverter.java
>  b1e17c9b5b8c29a81792ebfc637a53564bdb60fe 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ShellConfigurationConverter.java
>  ab73dfd2f24cbbfa60a37d75e5057f3ff2a1c0fa 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/SparkConfigurationConverter.java
>  8827769ea84f8f08a79e5e2d0189e2d1ded4bd9d 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/SqoopConfigurationConverter.java
>  1d4f615af583f8b941e570f3e3f239979cc8f825 
>   
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java
>  ec56554ba4cd313a9d5ab8472ab3c62b6bc17380 
>   fluent-job/fluent-job-api/src/main/resources/action_mappings.xml 
> a5f890eef9ccc352a5a852781f4212d96913e6d2 
>   fluent-job/fluent-job-api/src/main/xjb/bindings.xml 
> 48f68905ac2df2818d88907b26e863326d6d2c61 
>   
> fluent-job/fluent-job-client/src/test/java/org/apache/oozie/jobs/client/minitest/TestGitAction.java
>  PRE-CREATION 
>   pom.xml e0dbe85e5238fdc11cbd44cc31f65fc768c7454d 
>   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 39cea257e750eb8b3f3c4cb3f137093fccc03016 
>   src/main/assemblies/sharelib.xml 07dc69c8276895b254e5af0cc021cce6ebad18f4 
>   webapp/pom.xml fd3f89feedd76657f1431898a268298549826d6f 
> 
> 
> Diff: https://reviews.apache.org/r/68505/diff/1/
> 
> 
> Testing
> -------
> 
> Multiple unit and integration tests, as well as tests performed on a local 
> Oozie server w/ pseudo-distributed Hadoop.
> 
> Based on the work Clay B. has done in previous patchsets till 
> [OOZIE-2877.014-3.patch](https://issues.apache.org/jira/secure/attachment/12934336/OOZIE-2877.014-3.patch).
> 
> 
> Thanks,
> 
> András Piros
> 
>

Reply via email to