8285400: Add '@apiNote' to the APIs defined in Java SE 8 MR 3 #7

poonamparhar · 2022-04-25T21:02:05Z

This change adds "apiNote This method is defined in Java SE 8 Maintenance Release 3." to the doc comments of the new APIs added with the following changesets in Java SE MR3 :

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8285400: Add '@APinote' to the APIs defined in Java SE 8 MR 3

Reviewers

Bradford Wetmore (@bradfordwetmore - Reviewer)
Mandy Chung (@mlchung - Reviewer) ⚠️ Review applies to 7cdcb51
Valerie Peng (@valeriepeng - Reviewer) ⚠️ Review applies to 7cdcb51

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk8u-ri pull/7/head:pull/7
$ git checkout pull/7

Update a local copy of the PR:
$ git checkout pull/7
$ git pull https://git.openjdk.java.net/jdk8u-ri pull/7/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7

View PR using the GUI difftool:
$ git pr show -t 7

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk8u-ri/pull/7.diff

bridgekeeper · 2022-04-25T21:14:11Z

👋 Welcome back poonam! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

mlbridge · 2022-04-25T21:20:10Z

Webrevs

valeriepeng · 2022-04-25T23:21:47Z

How about the SHA512_224 and SHA512_256 constants in java/security/spec/MGF1ParameterSpec.java? Should add the since and apiNot javadoc tags as well?

bradfordwetmore

Is the word choice confirmed/approved, or still up for editing? Seems like this should be in past tense, IMHO.

This method was added in Java SE 8 Maintenance Release 3.

bradfordwetmore · 2022-04-25T23:33:17Z

Shouldn't the copyright dates be updated to 2022? These are significant changes to the files involved.

valeriepeng · 2022-04-25T23:33:25Z

jdk/src/share/classes/java/security/Signature.java

     * supposed to be used for digital signatures, an
     * {@code InvalidKeyException} is thrown.
     *
+     * @apiNote This method is defined in Java SE 8 Maintenance Release 3.


Hmm, there are other pkg private methods added in the maintenance release which do not have the @since javadoc tag. For consistency sake, maybe removing the @since tag here and no need to add @apiNote then?

I apparently let this slip through, this @since should not have been there. Please remove it and the @apiNote. Thanks.

valeriepeng · 2022-04-25T23:45:12Z

How about the SHA512_224 and SHA512_256 constants in java/security/spec/MGF1ParameterSpec.java? Should add the since and apiNot javadoc tags as well?

The PSSParameterSpec.TRAILER_FIELD_BC needs the @apiNote too?

bradfordwetmore · 2022-04-25T23:46:10Z

jdk/src/share/classes/javax/net/ssl/SSLEngine.java

-     * @apiNote
-     * This method should be called by TLS server applications before the TLS
+     * @apiNote This method is defined in Java SE 8 Maintenance Release 3.
+     *


IMHO, this needs a separator here to separate the existing API from the addition. Or maybe a second @apiNote? It currently renders as this:

API Note: This method is defined in Java SE 8 Maintenance Release 3. This method should be called by TLS server application...

I suggest using <p> to start with a new paragraph.

I'm guessing @mlchung meant <p>. Her suggestion didn't render correctly.

Yes, that's what I meant. I updated the formatting of my comment.

FYI: a second @apiNote does not render correctly, suggest using
which I've verified does.

API Note: This method is defined in Java SE 8 Maintenance Release 3. This method should be called by TLS server...

bradfordwetmore · 2022-04-25T23:51:02Z

jdk/src/share/classes/javax/net/ssl/SSLSocket.java

     *
-     * @apiNote
-     * This method should be called by TLS server applications before the TLS
+     * @apiNote This method is defined in Java SE 8 Maintenance Release 3.


Same concern here about either a second @apiNote or a separator.

bradfordwetmore · 2022-04-26T00:10:56Z

Same concern as @valeriepeng on both the MGF1ParameterSpec and PSSParameterSpec field omissions. We did add a @since in PSSParameterSpec, but not MGF1ParameterSpec.

poonamparhar · 2022-04-26T18:53:15Z

@bradfordwetmore @valeriepeng I have addressed all the concerns and have updated the PR. Please take a look.

bradfordwetmore

Copyright date updates not complete.

mlchung

This change looks good to me. Thanks for doing this update.

Can you file a JBS issue to fix in the mainline to add back @since in the constants in MGF1ParameterSpec class? Thanks.

bradfordwetmore · 2022-04-26T20:22:58Z

This change looks good to me. Thanks for doing this update.

Can you file a JBS issue to fix in the mainline to add back @since in the constants in MGF1ParameterSpec class? Thanks.

Good idea, thanks @mlchung!

bradfordwetmore

LGTM.

openjdk · 2022-04-29T16:39:24Z

@poonamparhar This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8285400: Add '@apiNote' to the APIs defined in Java SE 8 MR 3

Reviewed-by: wetmore, mchung, valeriep

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 4 new commits pushed to the master branch:

58ec25d: 8201793: (ref) Reference object should not support cloning
330d63d: 8175797: (ref) Reference::enqueue method should clear the reference object before enqueuing
ad6cdea: 8071507: (ref) Clear phantom reference as soft and weak references do
d63ed0a: 8284907: Compact profiles build fails with GNU make 4

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

poonamparhar · 2022-04-29T16:40:46Z

/integrate

openjdk · 2022-04-29T16:41:50Z

Going to push as commit 3740d05.
Since your change was applied there have been 4 commits pushed to the master branch:

58ec25d: 8201793: (ref) Reference object should not support cloning
330d63d: 8175797: (ref) Reference::enqueue method should clear the reference object before enqueuing
ad6cdea: 8071507: (ref) Clear phantom reference as soft and weak references do
d63ed0a: 8284907: Compact profiles build fails with GNU make 4

Your commit was automatically rebased without conflicts.

openjdk · 2022-04-29T16:42:10Z

@poonamparhar Pushed as commit 3740d05.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8285400: Add '@APinote' to the APIs defined in Java SE 8 MR 3

ead32fc

openjdk bot added the rfr Pull request is ready for review label Apr 25, 2022

bradfordwetmore reviewed Apr 25, 2022

View reviewed changes

valeriepeng reviewed Apr 25, 2022

View reviewed changes

bradfordwetmore reviewed Apr 25, 2022

View reviewed changes

addressed the review comments

7cdcb51

bradfordwetmore reviewed Apr 26, 2022

View reviewed changes

mlchung approved these changes Apr 26, 2022

View reviewed changes

valeriepeng approved these changes Apr 26, 2022

View reviewed changes

updated copyrights

0006dac

bradfordwetmore approved these changes Apr 26, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Apr 29, 2022

openjdk bot added the integrated Pull request has been integrated label Apr 29, 2022

openjdk bot closed this Apr 29, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 29, 2022

8285400: Add '@apiNote' to the APIs defined in Java SE 8 MR 3 #7

8285400: Add '@apiNote' to the APIs defined in Java SE 8 MR 3 #7

Uh oh!

Conversation

poonamparhar commented Apr 25, 2022 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Apr 25, 2022

Uh oh!

mlbridge bot commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

valeriepeng commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bradfordwetmore left a comment

Choose a reason for hiding this comment

Uh oh!

bradfordwetmore commented Apr 25, 2022

Uh oh!

valeriepeng Apr 25, 2022

Choose a reason for hiding this comment

Uh oh!

bradfordwetmore Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

valeriepeng commented Apr 25, 2022

Uh oh!

bradfordwetmore Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlchung Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bradfordwetmore Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlchung Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

bradfordwetmore Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

bradfordwetmore Apr 25, 2022

Choose a reason for hiding this comment

Uh oh!

bradfordwetmore commented Apr 26, 2022

Uh oh!

poonamparhar commented Apr 26, 2022

Uh oh!

bradfordwetmore left a comment

Choose a reason for hiding this comment

Uh oh!

mlchung left a comment

Choose a reason for hiding this comment

Uh oh!

bradfordwetmore commented Apr 26, 2022

Uh oh!

bradfordwetmore left a comment

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Apr 29, 2022

Uh oh!

poonamparhar commented Apr 29, 2022

Uh oh!

openjdk bot commented Apr 29, 2022

Uh oh!

openjdk bot commented Apr 29, 2022

Uh oh!

Reviewers

Assignees

Labels

poonamparhar commented Apr 25, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Apr 25, 2022 •

edited

Loading

valeriepeng commented Apr 25, 2022 •

edited

Loading

bradfordwetmore Apr 25, 2022 •

edited

Loading

mlchung Apr 26, 2022 •

edited

Loading

bradfordwetmore Apr 26, 2022 •

edited

Loading