jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8342827: Fix order of @param tags in other modules

Open hns opened this issue 1 year ago • 10 comments

Please review a doc-only change to fix the order of javadoc @param tags in in various OpenJDK modules. This is the third and last PR to fix the order of @param tags in OpenJDK libraries.

We are working on a javadoc feature to add an opt-in doclint warning for @param tags that don't match the order of parameters in the documented element. The warning will be enabled in OpenJDK builds and covers all uses of the @param tag, i.e. parameters of executable elements, type parameters, and record components.

I compared the generated API docs built with this branch with API docs built from master branch to make sure they are identical.


Progress

  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue
  • [ ] Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

  • JDK-8342827: Fix order of @param tags in other modules (Sub-task - P4)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21637

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

Using diff file

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

Webrev

Link to Webrev Comment

hns avatar Oct 22 '24 13:10 hns

:wave: 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.

bridgekeeper[bot] avatar Oct 22 '24 13:10 bridgekeeper[bot]

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

8342827: Fix order of @param tags in other modules

Reviewed-by: jpai, iris, joehw, darcy, dfuchs

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

  • d1540e2a49c7a41eb771fc9896c367187d070dec: 8342090: Infer::IncorporationBinaryOp::equals can produce side-effects
  • 7af46a6b424cadfe298958d774da0f21db58ecd3: 8340554: Improve MessageFormat readObject checks
  • 7d5eefa50673d6f7c5bd916f63271cf7898d6dee: 8342862: Gtest added by 8339507 appears to be causing 8GB build machines to hang
  • d8c3b0f834c603fe115ef4ca442727948b7a834e: 8342768: GTest AssemblerX86.validate_vm failed: assert(VM_Version::supports_bmi1()) failed: tzcnt instruction not supported
  • 3c14c2babbdfb46a77636ed80e083ef2f8be2b45: 8341566: Add Reader.of(CharSequence)
  • b0ac633b2d0076d64b463b2a6ce19abf6b12c50f: 8342075: HttpClient: improve HTTP/2 flow control checks
  • 85774b713edf8782f162ac25b61ce99a77e116f4: 8342882: RISC-V: Unify handling of jumps to runtime
  • 2c31c8eeb42188ad6fd15eca50db4342cd791fb2: 8339730: Windows regression after removing ObjectMonitor Responsible
  • f0b130e54f33d3190640ce33c991e35f27e9f812: 8339296: Record deconstruction pattern in switch fails to compile
  • e96b4cf0a81914c6a615bb4f62ea3f139a4737f3: 8342387: C2 SuperWord: refactor and improve compiler/loopopts/superword/TestDependencyOffsets.java
  • ... and 62 more: https://git.openjdk.org/jdk/compare/680dc5d896f4f7b01b3cf800d548e32bb2ef8c81...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[bot] avatar Oct 22 '24 13:10 openjdk[bot]

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

  • compiler
  • core-libs
  • net
  • 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.

openjdk[bot] avatar Oct 22 '24 13:10 openjdk[bot]

Webrevs

mlbridge[bot] avatar Oct 22 '24 13:10 mlbridge[bot]

Hello Hannes, since @param requires a parameter name and since javadoc already warns about incorrect @param names (that don't match the name of the method parameter), does the order in which the @param is listed in the javadoc text matter? I looked at https://bugs.openjdk.org/browse/JDK-8279931 but that doesn't tell why the order would be important.

jaikiran avatar Oct 22 '24 15:10 jaikiran

@jaikiran it does not matter for the generated documentation because we already put the tags in the correct order. So it's purely a code hygiene issue to make doc comments easier to read and maintain.

hns avatar Oct 22 '24 16:10 hns

Changes to java.management, java.naming, and sctp look good to me.

Thanks @dfuch, setting reviewers to 2 for changes in the the remaining modules.

/reviewers 2

hns avatar Oct 22 '24 16:10 hns

/reviewers 2

hns avatar Oct 22 '24 16:10 hns

@hns 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 1 Reviewer, 1 Author).

openjdk[bot] avatar Oct 22 '24 16:10 openjdk[bot]

@hns 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 1 Reviewer, 1 Author).

openjdk[bot] avatar Oct 22 '24 17:10 openjdk[bot]

Thanks all!

/integrate

hns avatar Oct 25 '24 07:10 hns

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

  • 415d8151a6c7ce04d8ab27ff641acb5d69840125: 8342938: Problem list java/io/IO/IO.java test on Linux ppc64le
  • c5489866308bd09729c5431537e0836145452624: 8342918: Building ParallelGC without G1 and Shenandoah fails because SpaceMangler not declared
  • ca1700b8bf3b7b34616c469837748ac4c3607dfc: 8336665: CCE in X509CRLImpl$TBSCertList.getCertIssuer
  • d1540e2a49c7a41eb771fc9896c367187d070dec: 8342090: Infer::IncorporationBinaryOp::equals can produce side-effects
  • 7af46a6b424cadfe298958d774da0f21db58ecd3: 8340554: Improve MessageFormat readObject checks
  • 7d5eefa50673d6f7c5bd916f63271cf7898d6dee: 8342862: Gtest added by 8339507 appears to be causing 8GB build machines to hang
  • d8c3b0f834c603fe115ef4ca442727948b7a834e: 8342768: GTest AssemblerX86.validate_vm failed: assert(VM_Version::supports_bmi1()) failed: tzcnt instruction not supported
  • 3c14c2babbdfb46a77636ed80e083ef2f8be2b45: 8341566: Add Reader.of(CharSequence)
  • b0ac633b2d0076d64b463b2a6ce19abf6b12c50f: 8342075: HttpClient: improve HTTP/2 flow control checks
  • 85774b713edf8782f162ac25b61ce99a77e116f4: 8342882: RISC-V: Unify handling of jumps to runtime
  • ... and 65 more: https://git.openjdk.org/jdk/compare/680dc5d896f4f7b01b3cf800d548e32bb2ef8c81...master

Your commit was automatically rebased without conflicts.

openjdk[bot] avatar Oct 25 '24 07:10 openjdk[bot]

@hns Pushed as commit fd5ff0547ced6733ae05f1428664062615408dc9.

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

openjdk[bot] avatar Oct 25 '24 07:10 openjdk[bot]