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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade dependencies, Upgrade to JDK17 #2747
Upgrade dependencies, Upgrade to JDK17 #2747
Changes from 3 commits
32b77b9
0c30885
7c62c05
66dceb6
0d702ac
c5cf52a
f7cb60a
ea77e9c
ea64779
bb52d58
a8dc5a3
8471415
f686ca6
dc83220
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same goes for this as well. Any specific reason behind us removing off this test that works with a custom security manager?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed it because of https://openjdk.java.net/jeps/411 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hduerkop - Please revert it back since we are still saying that we are going to be JDK11 as the minimum JDK but the security manager has been deprecated in JDK17. So we should perhaps leave it there as is until TestNG moves into JDK17 or above as its minimum JDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to disagree here since JDK17 is the current LTS and is or soon will be dominant on build platforms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hduerkop - if testng has jdk11 as its minimum jdk then we will need to have everything that exists in jdk11 including tests that test features which are relevant for jdk11 as well.
I agree that build systems atleast on the open source world may move forward but we need to be cognizant of user needs and cannot enforce this upon our users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not an enforcement, but an option to use JDK17 this way - 11 and 17 work this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the initial objective of the test? Is it another way to test it?