Todd Lipcon has posted comments on this change. Change subject: WIP: Add gradle build ......................................................................
Patch Set 2: (11 comments) http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradle/dependencies.gradle File java/gradle/dependencies.gradle: Line 8: avro : "1.8.1", ? http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradle/protobuf.gradle File java/gradle/protobuf.gradle: Line 14: // Configure Intellij to see the generated classes do we need the same for eclipse? http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradlew File java/gradlew: Line 1: #!/usr/bin/env sh license http://gerrit.cloudera.org:8080/#/c/7258/2/java/gradlew.bat File java/gradlew.bat: Line 1: @if "%DEBUG%" == "" @echo off license http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-flume-sink/build.gradle File java/kudu-flume-sink/build.gradle: Line 1: apply plugin: "com.commercehub.gradle.plugin.avro" we don't use avro do we? http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-jepsen/build.gradle File java/kudu-jepsen/build.gradle: Line 1: apply plugin: "nebula.clojure" add license header PS2, Line 7: if (it.project == project) { : it.onlyIf { false } : } is this equivalent to: it.onlyIf { it.project != project } ? PS2, Line 38: masterNodes does this second masterNodes somehow become equivalent to project.property("masterNodes")? should we be adding 'extra properties' as described in this doc? https://docs.gradle.org/3.3/userguide/writing_build_scripts.html It looks like that could handle the setting of defaults and also prevent against typos? http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-mapreduce/build.gradle File java/kudu-mapreduce/build.gradle: Line 1: dependencies { add license header. same in all other files http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala File java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala: Line 43: object ITBigLinkedList { why this rename? should we also rename the file then? is this incompatible if we expect users to be able to run ITBLL via submitting the kudu-spark-tools jar? http://gerrit.cloudera.org:8080/#/c/7258/2/java/kudu-spark/build.gradle File java/kudu-spark/build.gradle: Line 43: } can we add an 'else' here that fails the build to prevent people from making innocent mistakes like setting sparkBase=2.2? -- 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: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke <[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
