You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2018/04/26 02:46:44 UTC

[kudu-CR] [Java] Check in the Gradle wrapper properties

Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10215


Change subject: [Java] Check in the Gradle wrapper properties
......................................................................

[Java] Check in the Gradle wrapper properties

We were downloading the properties file, but that can result in
using and outdated Gradle wrapper distribution.

This changes .gitignore so the generated
gradle-wrapper.properties file is checked in anytime
we change the gradle verison.

I also added the silent (-s) flag to the curl command
when downloading the jar to remove some noise from
the log.

Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
---
M java/.gitignore
M java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
4 files changed, 8 insertions(+), 13 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/10215/1
-- 
To view, visit http://gerrit.cloudera.org:8080/10215
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
Gerrit-Change-Number: 10215
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>

[kudu-CR] [Java] Check in the Gradle wrapper properties

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10215

to look at the new patch set (#3).

Change subject: [Java] Check in the Gradle wrapper properties
......................................................................

[Java] Check in the Gradle wrapper properties

We were downloading the properties file, but that can result in
using an outdated Gradle wrapper distribution.

This changes .gitignore so the generated
gradle-wrapper.properties file is checked in anytime
we change the gradle verison and updates wrapper.gradle
to add a license to the header.

I also added the silent (-s) flag to the curl command
when downloading the jar to remove some noise from
the log.

Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
---
M java/.gitignore
M java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
4 files changed, 56 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/10215/3
-- 
To view, visit http://gerrit.cloudera.org:8080/10215
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
Gerrit-Change-Number: 10215
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [Java] Check in the Gradle wrapper properties

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10215 )

Change subject: [Java] Check in the Gradle wrapper properties
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10215/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10215/1//COMMIT_MSG@8
PS1, Line 8: 
           : We were downloading the properties file, but that can result in
           : using and outdated Gradle wrapper distribution.
> Given that downloading the properties file was intentional (see commit cc0b
The gradle wrapper jar doesn't really change from version to version and is only used to define how to download the distribution. So it's okay to lazily fetch that jar even when our gradle version changes.

The distribution to use is defined by the wrapper properties file. If the user has used an older version and then pulls in a new update that changes the gradle version, the code would see the properties file exists and not download the new one. This would leave the wrapper running an older version than the build defines. 

We download the jar because we shouldn't have binary files checked in. But we should check in the properties file because the build version used should be defined by the checked in code.

The other issue is the downloaded properties file is always pointing to the last release candidate for gradle instead of the release itself (due to the gradle release process I suppose). This is likely functionally equivalent but not optimal.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
Gerrit-Change-Number: 10215
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 27 Apr 2018 03:46:28 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Java] Check in the Gradle wrapper properties

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10215

to look at the new patch set (#2).

Change subject: [Java] Check in the Gradle wrapper properties
......................................................................

[Java] Check in the Gradle wrapper properties

We were downloading the properties file, but that can result in
using an outdated Gradle wrapper distribution.

This changes .gitignore so the generated
gradle-wrapper.properties file is checked in anytime
we change the gradle verison.

I also added the silent (-s) flag to the curl command
when downloading the jar to remove some noise from
the log.

Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
---
M java/.gitignore
M java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
4 files changed, 8 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/10215/2
-- 
To view, visit http://gerrit.cloudera.org:8080/10215
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
Gerrit-Change-Number: 10215
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [Java] Check in the Gradle wrapper properties

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10215 )

Change subject: [Java] Check in the Gradle wrapper properties
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10215/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10215/1//COMMIT_MSG@8
PS1, Line 8: 
           : We were downloading the properties file, but that can result in
           : using and outdated Gradle wrapper distribution.
Given that downloading the properties file was intentional (see commit cc0b19e), could you elaborate on how exactly this can happen?

Nit: "an"



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
Gerrit-Change-Number: 10215
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 27 Apr 2018 03:34:35 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Java] Check in the Gradle wrapper properties

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10215 )

Change subject: [Java] Check in the Gradle wrapper properties
......................................................................

[Java] Check in the Gradle wrapper properties

We were downloading the properties file, but that can result in
using an outdated Gradle wrapper distribution.

This changes .gitignore so the generated
gradle-wrapper.properties file is checked in anytime
we change the gradle verison and updates wrapper.gradle
to add a license to the header.

I also added the silent (-s) flag to the curl command
when downloading the jar to remove some noise from
the log.

Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
Reviewed-on: http://gerrit.cloudera.org:8080/10215
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M java/.gitignore
M java/gradle/wrapper.gradle
A java/gradle/wrapper/gradle-wrapper.properties
M java/gradlew
4 files changed, 56 insertions(+), 30 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Adar Dembo: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
Gerrit-Change-Number: 10215
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [Java] Check in the Gradle wrapper properties

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10215 )

Change subject: [Java] Check in the Gradle wrapper properties
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a772d5e093be29fcbabcbd8324bd6434fb33beb
Gerrit-Change-Number: 10215
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 27 Apr 2018 16:25:06 +0000
Gerrit-HasComments: No