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 2019/04/12 21:52:42 UTC
[kudu-CR] [build] Fix the java compatibility checker script
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13004
Change subject: [build] Fix the java compatibility checker script
......................................................................
[build] Fix the java compatibility checker script
This patch fixes check_compatibility.py. Primarily it
changed the existing script to use the Gradle build
in place of the Maven build that no longer exists.
It now uses japicmp in place of java_acc because
java_acc had issues unzipping the jars created by the
kudu build. Additionally japicmp’s filtering seemed to
work better.
The patch also updates the Gradle build to ensure
generated Protobuf classes are annotated with
@Generated to make report filtering easier.
Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
---
M build-support/check_compatibility.py
M java/gradle/protobuf.gradle
2 files changed, 84 insertions(+), 87 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13004/1
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Lieber-Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13004
to look at the new patch set (#3).
Change subject: [build] Fix the java compatibility checker script
......................................................................
[build] Fix the java compatibility checker script
This patch fixes check_compatibility.py. Primarily it
changed the existing script to use the Gradle build
in place of the Maven build that no longer exists.
It now uses japicmp in place of java_acc because
java_acc had issues unzipping the jars created by the
kudu build. Additionally japicmp’s filtering seemed to
work better.
The patch also updates the Gradle build to ensure
generated Protobuf classes are annotated with
@Generated to make report filtering easier.
In follow on patches I may incorparate this into the LINT job
and cause failure on API breakage.
Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
---
M build-support/check_compatibility.py
M java/gradle/protobuf.gradle
2 files changed, 108 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13004/3
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Lieber-Dembo <ad...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13004/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13004/3//COMMIT_MSG@22
PS3, Line 22: incorparate
> incorporate ?
Done
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py
File build-support/check_compatibility.py:
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@109
PS3, Line 109: if os.path.exists(get_japicmp_path()):
: logging.info("japicmp is already downloaded.")
> Will it work as expected when a new version of the tool is available? Mayb
Done
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@115
PS3, Line 115: "curl", "-o"
> nit: does it make sense to add few options to match the behavior of downloa
Done
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@121
PS3, Line 121: return [j for j in all_jars if (
> Nit: if I am reading this correctly, this code is excluding 9 patterns in o
I exclude jars because they are know to be non-public or application jars which do not have public API. I prefer a exclusion to explicit inclusion so that by default we scan and check APIs if a new jar is added.
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@175
PS3, Line 175: # TODO(ghenke): Add support for --error-on-binary-incompatibility and --error-on-source-incompatibility.
> Currently, the script produces a lot of output for modified and added metho
Currently the script uses super rudimentary arguments. I will enhance the arguments in a follow on patch once I understand what types of options a scheduled job might need.
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 11 Jan 2021 17:13:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
[build] Fix the java compatibility checker script
This patch fixes check_compatibility.py. Primarily it
changed the existing script to use the Gradle build
in place of the Maven build that no longer exists.
It now uses japicmp in place of java_acc because
java_acc had issues unzipping the jars created by the
kudu build. Additionally japicmp’s filtering seemed to
work better.
The patch also updates the Gradle build to ensure
generated Protobuf classes are annotated with
@Generated to make report filtering easier.
In follow on patches I may incorporate this into the LINT job
and cause failure on API breakage.
Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Reviewed-on: http://gerrit.cloudera.org:8080/13004
Tested-by: Kudu Jenkins
Reviewed-by: Greg Solovyev <gs...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M build-support/check_compatibility.py
M java/gradle/protobuf.gradle
2 files changed, 111 insertions(+), 91 deletions(-)
Approvals:
Kudu Jenkins: Verified
Greg Solovyev: Looks good to me, but someone else must approve
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13004/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13004/1//COMMIT_MSG@13
PS1, Line 13:
> Nit: extra space here.
Done
http://gerrit.cloudera.org:8080/#/c/13004/1/build-support/check_compatibility.py
File build-support/check_compatibility.py:
http://gerrit.cloudera.org:8080/#/c/13004/1/build-support/check_compatibility.py@204
PS1, Line 204: # Run the build in each tree.
> Hmm, would be nice if this would work on existing build output, since it'll
I agree that would be a good enhancement for follow on changes. For now, given the gradle build has been around a while, this is better than nothing and can be use for releases and arbitrary commits.
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Lieber-Dembo <ad...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 07 Jan 2021 21:20:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Greg Solovyev (Code Review)" <ge...@cloudera.org>.
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
Patch Set 4: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py
File build-support/check_compatibility.py:
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@121
PS3, Line 121: def find_client_jars(path):
> I exclude jars because they are know to be non-public or application jars w
Done
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@175
PS3, Line 175: if src_name == dst_name:
> Currently the script uses super rudimentary arguments. I will enhance the a
Done
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 11 Jan 2021 18:28:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Greg Solovyev (Code Review)" <ge...@cloudera.org>.
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py
File build-support/check_compatibility.py:
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@175
PS3, Line 175: # TODO(ghenke): Add support for --error-on-binary-incompatibility and --error-on-source-incompatibility.
Currently, the script produces a lot of output for modified and added methods, which do not break compatibility. In order to see if anything is broken, I had to edit the script and add --only-incompatible. Perhaps, a simpler way is to pass CLI options through to the japi?
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 08 Jan 2021 22:55:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Greg Solovyev (Code Review)" <ge...@cloudera.org>.
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py
File build-support/check_compatibility.py:
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@121
PS3, Line 121: return [j for j in all_jars if (
Nit: if I am reading this correctly, this code is excluding 9 patterns in order to find 6 files. Wouldn't it be easier include instead of exclude?
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 08 Jan 2021 22:44:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Greg Solovyev, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13004
to look at the new patch set (#4).
Change subject: [build] Fix the java compatibility checker script
......................................................................
[build] Fix the java compatibility checker script
This patch fixes check_compatibility.py. Primarily it
changed the existing script to use the Gradle build
in place of the Maven build that no longer exists.
It now uses japicmp in place of java_acc because
java_acc had issues unzipping the jars created by the
kudu build. Additionally japicmp’s filtering seemed to
work better.
The patch also updates the Gradle build to ensure
generated Protobuf classes are annotated with
@Generated to make report filtering easier.
In follow on patches I may incorporate this into the LINT job
and cause failure on API breakage.
Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
---
M build-support/check_compatibility.py
M java/gradle/protobuf.gradle
2 files changed, 111 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13004/4
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed Adar Lieber-Dembo from this change. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
Removed reviewer Adar Lieber-Dembo.
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 11 Jan 2021 21:54:00 +0000
Gerrit-HasComments: No
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
Patch Set 1:
(2 comments)
What are the next steps once this is merged?
http://gerrit.cloudera.org:8080/#/c/13004/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13004/1//COMMIT_MSG@13
PS1, Line 13:
Nit: extra space here.
http://gerrit.cloudera.org:8080/#/c/13004/1/build-support/check_compatibility.py
File build-support/check_compatibility.py:
http://gerrit.cloudera.org:8080/#/c/13004/1/build-support/check_compatibility.py@204
PS1, Line 204: # Run the build in each tree.
Hmm, would be nice if this would work on existing build output, since it'll be a lot easier to run it on historical artifacts that way (i.e. no need to worry about releases done before we switched to Gradle).
Would it be possible to allow for URLs or revisions, and if the former, download the JAR and use it directly?
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 14 Apr 2019 17:52:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Lieber-Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13004
to look at the new patch set (#2).
Change subject: [build] Fix the java compatibility checker script
......................................................................
[build] Fix the java compatibility checker script
This patch fixes check_compatibility.py. Primarily it
changed the existing script to use the Gradle build
in place of the Maven build that no longer exists.
It now uses japicmp in place of java_acc because
java_acc had issues unzipping the jars created by the
kudu build. Additionally japicmp’s filtering seemed to
work better.
The patch also updates the Gradle build to ensure
generated Protobuf classes are annotated with
@Generated to make report filtering easier.
Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
---
M build-support/check_compatibility.py
M java/gradle/protobuf.gradle
2 files changed, 108 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13004/2
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Lieber-Dembo <ad...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 )
Change subject: [build] Fix the java compatibility checker script
......................................................................
Patch Set 3: Code-Review+1
(3 comments)
Looks good overall, just a few nits.
http://gerrit.cloudera.org:8080/#/c/13004/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13004/3//COMMIT_MSG@22
PS3, Line 22: incorparate
incorporate ?
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py
File build-support/check_compatibility.py:
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@109
PS3, Line 109: if os.path.exists(get_japicmp_path()):
: logging.info("japicmp is already downloaded.")
Will it work as expected when a new version of the tool is available? Maybe, it makes sense to switch to a filename that matches the name of the downloaded JAR?
http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@115
PS3, Line 115: "curl", "-o"
nit: does it make sense to add few options to match the behavior of downloading source packages in thirdparty?
Something like
curl --retry 3 -L -o ...
--
To view, visit http://gerrit.cloudera.org:8080/13004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3
Gerrit-Change-Number: 13004
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 08 Jan 2021 22:38:55 +0000
Gerrit-HasComments: Yes