You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2017/08/31 22:56:32 UTC
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Hello Dan Burkert, Grant Henke,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/7927
to review the following change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
gradle: convert gradle-wrapper.properties into a real generated file
Let's handle gradle-wrapper.properties the same way as gradle-wrapper.jar:
by downloading it during invocations of gradlew if it's not already there.
I also put gradle/wrapper into .gitignore since it is only expected to
contain generated files.
Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
---
M java/.gitignore
M java/gradle/wrapper.gradle
D java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
4 files changed, 16 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/1
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7927/2/LICENSE.txt
File LICENSE.txt:
Line 563: java/gradlew: Apache 2.0 license
I think we should add something like:
This file is derived from code in the Gradle project,
copyright (c) the original author(s) and licensed under the
Apache 2.0 License.
http://gerrit.cloudera.org:8080/#/c/7927/2/java/gradlew
File java/gradlew:
Line 3: # Copyright 2017 the original author or authors.
how about: the original author or authors from the Gradle project.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
Patch Set 2: -Code-Review
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Mike Percy, Grant Henke, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7927
to look at the new patch set (#4).
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
gradle: convert the rest of the gradle wrapper into generated files
This patch replaces gradlew with a new script that downloads the real
gradlew (as well as the other gradle wrapper content) from GitHub before
invoking it. Thus we can treat the entirety of the wrapper (including
gradlew itself) as generated files.
Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
---
M java/.gitignore
M java/gradle/wrapper.gradle
D java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
4 files changed, 44 insertions(+), 214 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/4
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
looks good, just a comment nit
http://gerrit.cloudera.org:8080/#/c/7927/3/java/gradlew
File java/gradlew:
PS3, Line 40: # Run the real wrapper. By using 'source' we can trick the APP_HOME detection
: # in gradlew to think that APP_HOME=$JAVA_DIR. That way it'll naturally find
: # the gradle wrapper content in the gradle/wrapper subdirectory.
How about something like:
Run the "real" gradlew script. Even though we downloaded it into the wrapper/ directory, if we invoke it via 'source' it will be tricked into thinking it is running from the java root directory, which is where it expects to live. That way it'll naturally find the gradle wrapper content in the gradle/wrapper subdirectory.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
gradle: convert gradle-wrapper.properties into a real generated file
Let's handle gradle-wrapper.properties the same way as gradle-wrapper.jar:
by downloading it during invocations of gradlew if it's not already there.
I also put gradle/wrapper into .gitignore since it is only expected to
contain generated files.
Finally, I added an ASL2 header to gradlew and linked to it from Kudu's
central license file.
Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Reviewed-on: http://gerrit.cloudera.org:8080/7927
Reviewed-by: Mike Percy <mp...@apache.org>
Tested-by: Kudu Jenkins
---
M LICENSE.txt
M java/.gitignore
M java/gradle/wrapper.gradle
D java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
5 files changed, 31 insertions(+), 10 deletions(-)
Approvals:
Mike Percy: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7927/2/LICENSE.txt
File LICENSE.txt:
Line 563: java/gradlew: Apache 2.0 license
> 'this file'? Wouldn't that refer to LICENSE.txt in this context?
I think it's clear we are referring to java/gradlew here, based on text further up in LICENSE.txt such as "Some portions of this module are derived from code from LevelDB", however we could call it a module or a script instead.
http://gerrit.cloudera.org:8080/#/c/7927/2/java/gradlew
File java/gradlew:
Line 3: # Copyright 2017 the original author or authors.
> This is what a bunch of license headers look like in Gradle. We don't nece
OK. Let's leave this as-is then.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Grant Henke, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7927
to look at the new patch set (#5).
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
gradle: convert gradle-wrapper.properties into a real generated file
Let's handle gradle-wrapper.properties the same way as gradle-wrapper.jar:
by downloading it during invocations of gradlew if it's not already there.
I also put gradle/wrapper into .gitignore since it is only expected to
contain generated files.
Finally, I added an ASL2 header to gradlew and linked to it from Kudu's
central license file.
Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
---
M LICENSE.txt
M java/.gitignore
M java/gradle/wrapper.gradle
D java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
5 files changed, 31 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/5
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 4:
(2 comments)
re: "real distribution"
I mean from maven central or http://services.gradle.org/distributions/ instead of github.
The problem is they don't publish the gradle-wrapper jar as a standalone artifact since this isn't the usual pattern.
http://gerrit.cloudera.org:8080/#/c/7927/4/java/gradle/wrapper.gradle
File java/gradle/wrapper.gradle:
Line 29
Do we want to "override" the wrapper command to replace the gradlew with your script? That will ensure the gradle version in dependencies.gradle is still used.
My goal in making "gradle wrapper" still work was not to break user expectations. Especially those familiar with gradle.
http://gerrit.cloudera.org:8080/#/c/7927/4/java/gradlew
File java/gradlew:
Line 1: #!/bin/bash
We should test that this works with IDE integrations like IntelliJ. They expect this script, but I am not sure if they have other expectations.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Grant Henke, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7927
to look at the new patch set (#3).
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
gradle: convert the rest of the gradle wrapper into generated files
This patch replaces gradlew with a new script that downloads the real
gradlew (as well as the other gradle wrapper content) from GitHub before
invoking it. Thus we can treat the entirety of the wrapper (including
gradlew itself) as generated files.
Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
---
M java/.gitignore
M java/gradle/wrapper.gradle
D java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
4 files changed, 41 insertions(+), 214 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/3
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 3:
> I think this is okay for this release since the gradle build is
> experimental but we should use a real distribution link if possible
> in the future.
What do you mean by "real distribution link"? Can you elaborate?
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 4:
> Sorry on phone it won't let me post inthe thread. I don't see why
> the graders case needs to be explained further. It's no different
> than the have files a line above.
OK, your argument is convincing. I would be more comfortable if the included file(s) had copyright notices or something mentioning the Gradle project (in order to indicate provenance) but I suppose the fact that it's "gradlew" is clear enough where it came from and due to the ASL 2.0 license, attribution is not required. So +2 from me on Patch Set 2.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
Patch Set 5: Code-Review+2
I manually rebased Adar's Patch Set 2 onto master and re-pushed it to Gerrit.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7927/2/LICENSE.txt
File LICENSE.txt:
Line 563: java/gradlew: Apache 2.0 license
> I think we should add something like:
'this file'? Wouldn't that refer to LICENSE.txt in this context?
http://gerrit.cloudera.org:8080/#/c/7927/2/java/gradlew
File java/gradlew:
Line 3: # Copyright 2017 the original author or authors.
> how about: the original author or authors from the Gradle project.
This is what a bunch of license headers look like in Gradle. We don't necessarily know that the Gradle project holds copyright on this file.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7927/2/java/gradlew
File java/gradlew:
PS2, Line 86: https://raw.githubusercontent.com/gradle/gradle/v4.1.0/gradle/wrapper/gradle-wrapper.properties
nit: what if they put a redirect there in future? Consider adding '-L' option while invoking the curl utility.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7927/1/java/gradlew
File java/gradlew:
Line 3: ##############################################################################
What is the license of this file?
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7927/3/java/gradlew
File java/gradlew:
PS3, Line 40: # Run the real wrapper. By using 'source' we can trick the APP_HOME detection
: # in gradlew to think that APP_HOME=$JAVA_DIR. That way it'll naturally find
: # the gradle wrapper content in the gradle/wrapper subdirectory.
> How about something like:
Thanks. I used what you wrote modulo a few small changes.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 2:
Sorry on phone it won't let me post inthe thread. I don't see why the graders case needs to be explained further. It's no different than the have files a line above.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7927/1/java/gradlew
File java/gradlew:
Line 3: ##############################################################################
> What is the license of this file?
ASL presumably, since that's the license for all of Gradle.
The upstream version of gradlew (https://github.com/gradle/gradle/blob/master/gradlew) doesn't have a license header either, and this is a copy of it (modulo the curl bits that we "inject").
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: gradle: convert the rest of the gradle wrapper into generated files
......................................................................
Patch Set 3: Code-Review+1
I think this is okay for this release since the gradle build is experimental but we should use a real distribution link if possible in the future.
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Grant Henke, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7927
to look at the new patch set (#2).
Change subject: gradle: convert gradle-wrapper.properties into a real generated file
......................................................................
gradle: convert gradle-wrapper.properties into a real generated file
Let's handle gradle-wrapper.properties the same way as gradle-wrapper.jar:
by downloading it during invocations of gradlew if it's not already there.
I also put gradle/wrapper into .gitignore since it is only expected to
contain generated files.
Finally, I added an ASL2 header to gradlew and linked to it from Kudu's
central license file.
Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
---
M LICENSE.txt
M java/.gitignore
M java/gradle/wrapper.gradle
D java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
5 files changed, 31 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/2
--
To view, visit http://gerrit.cloudera.org:8080/7927
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>