Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12775 )
Change subject: [build] Make Gradle download URLs configurable ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/12775/1/java/build.gradle File java/build.gradle: http://gerrit.cloudera.org:8080/#/c/12775/1/java/build.gradle@101 PS1, Line 101: distributionUrl = "${gradleDistUrlBase}/gradle-${versions.gradle}-all.zip" This line should be added to the wrapper task in wrapper.gradle. http://gerrit.cloudera.org:8080/#/c/12775/1/java/gradle.properties File java/gradle.properties: http://gerrit.cloudera.org:8080/#/c/12775/1/java/gradle.properties@63 PS1, Line 63: gradleWrapperUrl = https://raw.githubusercontent.com/gradle/gradle/v%s/gradle/wrapper Can you add a comment that these are used in the `gradle wrapper` task to generate `gradlew` and the `wrapper/gradle-wrapper.properties`? http://gerrit.cloudera.org:8080/#/c/12775/1/java/gradle/wrapper.gradle File java/gradle/wrapper.gradle: http://gerrit.cloudera.org:8080/#/c/12775/1/java/gradle/wrapper.gradle@59 PS1, Line 59: def wrapperBaseUrl = String.format(gradleWrapperUrl, fullVersion) Interestingly the version of the gradle jar isn't really important. Instead of this, we can put the full url in the properties file. I would use the last major version: https://raw.githubusercontent.com/gradle/gradle/v5.0.0/gradle/wrapper/gradle-wrapper.jar Can you also move the comments from this section to the properties file. -- To view, visit http://gerrit.cloudera.org:8080/12775 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9317d8a84ccf788892163de1d89999e7749fdb34 Gerrit-Change-Number: 12775 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 18 Mar 2019 16:41:23 +0000 Gerrit-HasComments: Yes
