Grant Henke has posted comments on this change.

Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................


Patch Set 14:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/compile.gradle
File java/gradle/compile.gradle:

Line 24:     options.fork = true // enable compilation in a separate daemon 
process.
> I didn't mean to suggest it should be removed. I just didn't know whether i
I didn't have a great reason either. So removing it keeps things simple.


Line 31:     }
> You'd have to add a Maven profile. See https://stackoverflow.com/a/16743137
After spending more time on this and making the maven profile work. It turns 
out Kudu doesn't have too many errors to overcome like other projects I have 
worked on. I think I will drop this and fix the lint errors. It should make for 
cleaner javadoc in the long run.


http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/dependencies.gradle
File java/gradle/dependencies.gradle:

Line 60: versions["scalaBase"] = versions.scala.substring(0, 
versions.scala.lastIndexOf("."))
> Missed this?
Oh sorry meant to respond. If the Scala version doesn't match this expected 
pattern, the build would fail and it would show that the "malformed" version 
couldn't be resolved. Perhaps that is good enough?


http://gerrit.cloudera.org:8080/#/c/7258/16/java/gradle/quality.gradle
File java/gradle/quality.gradle:

Line 22: 
> for
Done


http://gerrit.cloudera.org:8080/#/c/7258/14/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:

Line 28:   doLast {
> Could you add a comment explaining that?
Done


http://gerrit.cloudera.org:8080/#/c/7258/16/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:

Line 27: task bootstrapWrapper() {
> Single quote was better here.
oops....a little trigger happy on the find and replace.


http://gerrit.cloudera.org:8080/#/c/7258/14/java/kudu-mapreduce/build.gradle
File java/kudu-mapreduce/build.gradle:

Line 17: 
> Hmm, ask Dan?
Will ask.


Line 23:   }
> Nice. Can you add a link to the pull request in each place where you apply 
Done


http://gerrit.cloudera.org:8080/#/c/7258/16/java/kudu-spark/pom.xml
File java/kudu-spark/pom.xml:

Line 108:                         <phase>process-resources</phase>
> I didn't see this in the gradle build. Is it built-in to gradle's scala plu
I was playing with the args and left these in. Fixed to match the gradle build 
in compile.gradle. I also added docs for the flags.


http://gerrit.cloudera.org:8080/#/c/7258/16/java/settings.gradle
File java/settings.gradle:

> File-level comment.
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to