TEZ-4677: Upgrades Tez to compile and run with JDK 25#482
Open
maheshrajus wants to merge 6 commits into
Open
Conversation
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
Author
|
It looks like the Jenkinsfile and Dockerfile have changed and been optimized in the latest commits. I need to rebase with the latest changes and check on this task. @Aggarwal-Raghav, if we want to test Tez with JDK 25, which fields in the latest Jenkinsfile and Dockerfile do we need to modify? As far as I can see in the latest changes, there are no fields related to the JDK version. |
Contributor
|
@maheshrajus , please check this.
|
maheshrajus
changed the title
|
(!) A patch to the testing environment has been detected. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
Contributor
|
@maheshrajus , now the spotbugs have failed because its incompatible with jdk25. I'll investigate it in some time to see which is better approach (maven-shading or apache pom version) from my perspective and share my findings as both have some tradeoffs in terms of error |
Contributor
|
This much of the patch should be enough. It is passing in local. Regarding Lines 49 to 53 in 17d1549 jdk25.patch |
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
| private Integer failAll = -1; | ||
|
|
||
| public Integer getFailAll() { | ||
| return failAll; |
Contributor
There was a problem hiding this comment.
Is this because of spotbugs/checkstyle?
Contributor
Author
There was a problem hiding this comment.
yes, checkstyle issues.
Contributor
|
LGTM +1 for pom, Dockerfile changes. JDK25 changes look good. For checkstyle plese consult with a committer/PMC whether to do in this PR or separate? |
Contributor
Author
|
@Aggarwal-Raghav All checkstyle issues related to jdk 25 so better to include in this PR itself. @abstractdog Could you please review the PR at your convenience? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.