8347123: Add missing @serial tags to other modules by hns · Pull Request #22980 · openjdk/jdk

hns · 2025-01-08T20:13:50Z

Please review a doc-only change to mostly add missing @serial javadoc tags. This is a sub-task of JDK-8286931 to allow us to re-enable the javadoc -serialwarn option in the JDK doc build, which has been disabled since JDK 19.

For private and package-private serialized fields that already have a doc comment, the main description is converted to a block tag by prepending @serial since these fields do not require a main description. For protected and public serialized fields that require a main description, an empty @serial block tag is appended to the doc comment instead. The effect is the same, as the main description is used in serial-form.html if the @serial tag is missing or empty. For those fields that do not have a doc comment, a doc comment with an empty @serial tag is added.

Apart from missing @serial tags, this PR also adds a @serialData tag to java.awt.datatransfer.DataFlavor.writeExternal(ObjectOutput) that the javadoc -serialwarn option complains about. This is the only change in this PR that adds documentation text and causes a change in the generated documentation.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 2 Reviewers)
Change requires CSR request JDK-8348408 to be approved

Issues

JDK-8347123: Add missing @serial tags to other modules (Sub-task - P4)
JDK-8348408: Add missing @serial tags to other modules (CSR)

Reviewers

Phil Race (@prrace - Reviewer)
Nizar Benalla (@nizarbenalla - Committer)
Alan Bateman (@AlanBateman - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22980/head:pull/22980
$ git checkout pull/22980

Update a local copy of the PR:
$ git checkout pull/22980
$ git pull https://git.openjdk.org/jdk.git pull/22980/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22980

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22980.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-01-08T20:14:52Z

👋 Welcome back hannesw! 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.

openjdk · 2025-01-08T20:16:02Z

@hns 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:

8347123: Add missing @serial tags to other modules

Reviewed-by: prr, nbenalla, alanb

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

8b2aa51: 8349780: AIX os::get_summary_cpu_info support Power 11
650d0d9: 8348865: JButton/bug4796987.java never runs because Windows XP is unavailable
906358d: 8294155: Exception thrown before awaitAndCheck hangs PassFailJFrame
2bd8f02: 8342524: Use latch in AbstractButton/bug6298940.java instead of delay
7f3ecb4: 8346664: C2: Optimize mask check with constant offset
b3a4026: 8349764: RISC-V: C1: Improve Class.isInstance intrinsic
071c8f5: 8349909: jdk.internal.jimage.decompressor.ZipDecompressor does not close the Inflater in exceptional cases
f1258f9: 8349755: Fix corner case issues in async UL
b1b4828: 8350086: Inline hot Method accessors for faster task selection
5e9d72e: 8350094: Linux gcc 13.2.0 build fails when ubsan is enabled
... and 517 more: https://git.openjdk.org/jdk/compare/a641932427cbe8453130593355372837d70a098f...master

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.

openjdk · 2025-01-08T20:16:41Z

@hns The following labels will be automatically applied to this pull request:

client
core-libs
kulla
security
serviceability

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.

mlbridge · 2025-01-08T20:30:11Z

Webrevs

01: Full - Incremental (b686fb10)
00: Full (c4be23a7)

prrace · 2025-01-21T19:41:39Z

src/java.datatransfer/share/classes/java/awt/datatransfer/DataFlavor.java

@@ -1286,6 +1286,9 @@ public boolean isFlavorTextType() {

    /**
     * Serializes this {@code DataFlavor}.


This most definitely changes the serialisation spec. So a CSR is needed.
Also shouldn't readExternal be updated to correspond ?

Only the writeExternal method is required to have a @serialData tag in order to keep javadoc running with -serialwarn option from complaining. I guess the thinking is that the readExternal logic can be derived from that.

@prrace do you have any suggestions for the spec change, or are you ok with the proposed wording?

There's not a great number of "good" examples of this in the JDK, so probably OK except it
seems like most cases will do it like a normal javadoc method so you'd want an @param tag too.

note : this is a desktop class, so I'm looking at this one but someone else will need to look at all the others

I've created a CSR with a slightly modified version of the text proposed in this PR:
https://bugs.openjdk.org/browse/JDK-8348408

Other doc tags such as @param and @throws are inherited from the overridden method. Also, this is the only change in this PR that modifies the serialization spec.

CSR was approved, and I updated the PR with the slightly modified spec text from the CSR.

prrace · 2025-01-21T19:41:55Z

/csr

openjdk · 2025-01-21T19:42:54Z

@prrace has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@hns please create a CSR request for issue JDK-8347123 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

prrace

Approving the datatransfer portion

prrace · 2025-01-28T20:14:10Z

/reviewers 2 reviewer

.. at least ...

openjdk · 2025-01-28T20:14:50Z

@prrace
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 2 Reviewers).

nizarbenalla

Code changes make sense and the updated text matches the text from the CSR.

AlanBateman

I think this is oky, the CSR for the change writeExternal has already been reviewed.

hns · 2025-02-17T15:30:49Z

/integrate

openjdk · 2025-02-17T15:31:23Z

Going to push as commit 3f0c137.
Since your change was applied there have been 527 commits pushed to the master branch:

8b2aa51: 8349780: AIX os::get_summary_cpu_info support Power 11
650d0d9: 8348865: JButton/bug4796987.java never runs because Windows XP is unavailable
906358d: 8294155: Exception thrown before awaitAndCheck hangs PassFailJFrame
2bd8f02: 8342524: Use latch in AbstractButton/bug6298940.java instead of delay
7f3ecb4: 8346664: C2: Optimize mask check with constant offset
b3a4026: 8349764: RISC-V: C1: Improve Class.isInstance intrinsic
071c8f5: 8349909: jdk.internal.jimage.decompressor.ZipDecompressor does not close the Inflater in exceptional cases
f1258f9: 8349755: Fix corner case issues in async UL
b1b4828: 8350086: Inline hot Method accessors for faster task selection
5e9d72e: 8350094: Linux gcc 13.2.0 build fails when ubsan is enabled
... and 517 more: https://git.openjdk.org/jdk/compare/a641932427cbe8453130593355372837d70a098f...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-02-17T15:31:30Z

@hns Pushed as commit 3f0c137.

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

8347123: Add missing @serial tags to other modules

c4be23a

openjdk bot added the rfr Pull request is ready for review label Jan 8, 2025

openjdk bot added serviceability serviceability-dev@openjdk.org security security-dev@openjdk.org client client-libs-dev@openjdk.org core-libs core-libs-dev@openjdk.org kulla kulla-dev@openjdk.org labels Jan 8, 2025

prrace reviewed Jan 21, 2025

View reviewed changes

openjdk bot added the csr Pull request needs approved CSR before integration label Jan 21, 2025

openjdk bot removed the csr Pull request needs approved CSR before integration label Jan 24, 2025

Update @Serialdata text to CSR-approved version

b686fb1

prrace approved these changes Jan 28, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 28, 2025

openjdk bot removed the ready Pull request is ready to be integrated label Jan 28, 2025

nizarbenalla approved these changes Jan 31, 2025

View reviewed changes

AlanBateman approved these changes Feb 17, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 17, 2025

openjdk bot added the integrated Pull request has been integrated label Feb 17, 2025

openjdk bot closed this Feb 17, 2025

openjdk bot removed the ready Pull request is ready to be integrated label Feb 17, 2025

openjdk bot removed the rfr Pull request is ready for review label Feb 17, 2025

		@@ -1286,6 +1286,9 @@ public boolean isFlavorTextType() {

		/**
		* Serializes this {@code DataFlavor}.

Conversation

hns commented Jan 8, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jan 8, 2025

Uh oh!

openjdk bot commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jan 8, 2025

Uh oh!

mlbridge bot commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

prrace Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

hns Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

prrace Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

hns Jan 23, 2025

Choose a reason for hiding this comment

Uh oh!

hns Jan 24, 2025

Choose a reason for hiding this comment

Uh oh!

prrace commented Jan 21, 2025

Uh oh!

openjdk bot commented Jan 21, 2025

Uh oh!

prrace left a comment

Choose a reason for hiding this comment

Uh oh!

prrace commented Jan 28, 2025

Uh oh!

openjdk bot commented Jan 28, 2025

Uh oh!

nizarbenalla left a comment

Choose a reason for hiding this comment

Uh oh!

AlanBateman left a comment

Choose a reason for hiding this comment

Uh oh!

hns commented Feb 17, 2025

Uh oh!

openjdk bot commented Feb 17, 2025

Uh oh!

openjdk bot commented Feb 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

hns commented Jan 8, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jan 8, 2025 •

edited

Loading

mlbridge bot commented Jan 8, 2025 •

edited

Loading