-
Notifications
You must be signed in to change notification settings - Fork 5.8k
8353197: Document preconditions for JavaLangAccess methods #24982
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
👋 Welcome back vyazici! A progress list of the required criteria for merging this PR into |
@vy 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 71 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@minborg, @liach) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
Webrevs
|
src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
Outdated
Show resolved
Hide resolved
src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
Outdated
Show resolved
Hide resolved
src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
Outdated
Show resolved
Hide resolved
Hello Volkan, I checked with others on how to make it prominent, when reading code and when reviewing changes to code, that the usage of these methods needs extra careful attention. In addition to updating the javadoc of these methods, we came to an agreement that each of these methods should be updated to have |
/reviewers 2 |
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.
LGTM
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.
Reviewed missing copyright updates. Only files with comments need copyright year updates.
src/jdk.charsets/share/classes/sun/nio/cs/ext/EUC_JP.java.template
Outdated
Show resolved
Hide resolved
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.
Let's re-approve after copyright fix.
@liach, @minborg, I've fixed the copyright years. I'd appreciate it if you can approve one last time. This last copyright year fiasco made me do this:
|
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.
Latest changes look good.
src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
Outdated
Show resolved
Hide resolved
src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
Outdated
Show resolved
Hide resolved
src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Roger Riggs <Roger.Riggs@Oracle.com>
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.
So, I think we are ready to go with the latest version.
Green /integrate |
/sponsor Thanks! |
Going to push as commit 8fcfddb.
Your commit was automatically rebased without conflicts. |
Document preconditions on certain
JavaLangAccess
methods that use operations either unsafe and/or without range checks.Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24982/head:pull/24982
$ git checkout pull/24982
Update a local copy of the PR:
$ git checkout pull/24982
$ git pull https://git.openjdk.org/jdk.git pull/24982/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 24982
View PR using the GUI difftool:
$ git pr show -t 24982
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24982.diff
Using Webrev
Link to Webrev Comment