-
Notifications
You must be signed in to change notification settings - Fork 5.8k
JDK-8296149: Start of release updates for JDK 21 #10924
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
Conversation
👋 Welcome back darcy! A progress list of the required criteria for merging this PR into |
@jddarcy The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
/issue add JDK-8296150 |
@jddarcy |
/issue add JDK-8296151 |
@jddarcy |
Flipping the draft-bit off for this PR ahead of the upcoming start of JDK 21. Symbol information now up-to-date as of JDK 20 b25. |
Webrevs
|
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 looked at everything bar the sym files. All seems fine/usual.
One query below (probably one I ask each release).
Thanks.
@@ -44,7 +44,7 @@ | |||
* @see AbstractAnnotationValueVisitor9 | |||
* @since 14 | |||
*/ | |||
@SupportedSourceVersion(RELEASE_20) | |||
@SupportedSourceVersion(RELEASE_21) |
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 a pity these have to be updated each release. Is there no way to say latest
here?
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.
Not with the current language specification, no.
The only enum value "constant-enough" to use in an annotation is the name of an enum constant, a public static final field assigned an enum value is not enough.
On the flip side, I make the updates to these annotations using a one-line sed script I have saved away in a comment on one of the previous start-of-release-updates-for-JDK-N bugs. Using the annotations in this way is in lieu of adding another type specification for the JDK version. In other words, having AbstractAnnotationValueVisitor14.java with an annotation indicating RELEASE_21 prevents having AbstractAnnotationValueVisitor15, AbstractAnnotationValueVisitor16, ... , AbstractAnnotationValueVisitor21.
New visitors are now only introduced when adding new kinds of structures requires differences in behavior.
Thanks for the review.
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.
looks good
@@ -88,7 +88,8 @@ static enum SourceTarget { | |||
SEVENTEEN(false, "61.0", "17", Versions::checksrc17), | |||
EIGHTEEN(false, "62.0", "18", Versions::checksrc18), | |||
NINETEEN(false, "63.0", "19", Versions::checksrc19), | |||
TWENTY(false, "64.0", "20", Versions::checksrc20); | |||
TWENTY(false, "64.0", "20", Versions::checksrc20), |
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.
nit:
TWENTY(false, "64.0", "20", Versions::checksrc20), | |
TWENTY(false, "64.0", "20", Versions::checksrc20), |
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.
javac changes look fine to me.
@jddarcy This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 5 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
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.
Still looks good.
/integrate delegate |
@jddarcy Integration of this pull request has been delegated and may be completed by any project committer using the /integrate pull request command. |
/integrate |
Going to push as commit 175e3d3.
Your commit was automatically rebased without conflicts. |
@JesperIRL Pushed as commit 175e3d3. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Usual start-of-release updates. Symbol updates in initial version reflect JDK 20 build 21.
Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10924/head:pull/10924
$ git checkout pull/10924
Update a local copy of the PR:
$ git checkout pull/10924
$ git pull https://git.openjdk.org/jdk pull/10924/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 10924
View PR using the GUI difftool:
$ git pr show -t 10924
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10924.diff