Skip to content
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

JDK-8286338: suppress warnings about bad @author tags when author info is not generated. #8583

Conversation

jonathan-gibbons
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons commented May 6, 2022

Please review a simple small change to TagletManager, to not report warnings about bad use of tags (such as @author and @version) when command-line options have not been given to enable the use of the tags in the output (i.e. the -author and the dreadfully-named -version option.)

The change is more pragmatic than anything else. There are lots of "bad" uses of @author in JDK API documentation, on methods. The alternative would be to permit the tag anywhere, but that would be both a spec change and work to implement the change that we're not really interest in, since the tag is now somewhat deprecated.

Two existing tests are updated, to test the handling of "bad" tags, with and without the enabling option.

Another test has some very (very) old unused lines deleted. If today was Thursday, I'd call it Throwback Thursday. These lines predate the JavadocTester conversion.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed (1 review required, with at least 1 reviewer)

Issue

  • JDK-8286338: suppress warnings about bad @author tags when author info is not generated.

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8583/head:pull/8583
$ git checkout pull/8583

Update a local copy of the PR:
$ git checkout pull/8583
$ git pull https://git.openjdk.java.net/jdk pull/8583/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8583

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8583.diff

@bridgekeeper
Copy link

bridgekeeper bot commented May 6, 2022

👋 Welcome back jjg! 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 openjdk bot added the rfr label May 6, 2022
@openjdk
Copy link

openjdk bot commented May 6, 2022

@jonathan-gibbons The following label will be automatically applied to this pull request:

  • javadoc

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the javadoc label May 6, 2022
@mlbridge
Copy link

mlbridge bot commented May 6, 2022

Webrevs

Copy link
Member

@pavelrappo pavelrappo left a comment

What's that -XDdummy=dummy you use in tests?

@@ -35,16 +35,6 @@

public class TestSimpleTagInherit extends JavadocTester {

//Javadoc arguments.
Copy link
Member

@pavelrappo pavelrappo May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like an unrelated change; is it cleanup?

Copy link
Contributor Author

@jonathan-gibbons jonathan-gibbons May 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is a holdover from the bad old days before JavadocTester, when tests were set up using fixed arrays like these. Frankly, I'm surprised these two empty arrays escaped notice for so long.

@@ -53,7 +54,7 @@ public static void main(String... args) throws Exception {
tb.writeJavaFiles(src,
"""
package pkg;
/** Introduction.\s
/** Introduction.
Copy link
Member

@pavelrappo pavelrappo May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup?

Copy link
Contributor Author

@jonathan-gibbons jonathan-gibbons May 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

@jonathan-gibbons
Copy link
Contributor Author

jonathan-gibbons commented May 12, 2022

What's that -XDdummy=dummy you use in tests?

It's a no-op placeholder. JavaDoc does not allow empty arguments, and the alternative is dynamically build a list with a conditional component. -XD is a "hidden" option to inject values in the compiler options map, so -XDdummy=dummy just injects a dummy value there.

@pavelrappo
Copy link
Member

pavelrappo commented May 13, 2022

What's that -XDdummy=dummy you use in tests?

It's a no-op placeholder. JavaDoc does not allow empty arguments, and the alternative is dynamically build a list with a conditional component. -XD is a "hidden" option to inject values in the compiler options map, so -XDdummy=dummy just injects a dummy value there.

Okay, I can see what happens when you pass an empty string:

error: Illegal package name: ""

I wish there were a better way of conditionally providing a javadoc option.

@openjdk
Copy link

openjdk bot commented May 13, 2022

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

8286338: suppress warnings about bad @author tags when author info is not generated.

Reviewed-by: prappo

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

  • d5ae383: 8286117: Remove unnecessary indirection and unused code in UL
  • c3bade2: 8286623: Bundle zlib by default with JDK on macos aarch64
  • 617ef54: 8286647: JFR: Build failure when C1 or C2 is disabled after JDK-8282420
  • 369611e: 8286677: [BACKOUT] (fc) Tune FileChannel.transferFrom()
  • 4b8a66a: 8286424: GetVersionEx is deprecated
  • 986d87d: 8274113: (fc) Tune FileChannel.transferFrom()
  • 61cb4b7: 8285951: Replace Algorithms.eatMemory(...) with WB.fullGC() in vmTestbase_vm_gc_ref tests
  • 5ff1d22: 8286386: Address possibly lossy conversions in java.net.http
  • 7118343: 8278262: JFR: TestPrintXML can't handle missing timestamps
  • 74eee28: 8286560: Remove user parameter from jdk.internal.perf.Perf.attach()
  • ... and 102 more: https://git.openjdk.java.net/jdk/compare/080f3c5d8a2f7b2d13baf98c594d4ace67608fc4...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 openjdk bot added the ready label May 13, 2022
@jonathan-gibbons
Copy link
Contributor Author

jonathan-gibbons commented May 17, 2022

/integrate

@openjdk
Copy link

openjdk bot commented May 17, 2022

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

  • a25b9bc: 8286688: JFR: Active Setting events should have the same timestamp
  • 5bea461: 8286734: (fc) FileChannelImpl#map() cleanup after merge of Foreign Function & Memory API
  • 0c5ab6d: 8209038: Clarify the javadoc of Cipher.getParameters()
  • 1d8e92a: 8213045: Add BigDecimal.TWO
  • 8535d51: 8286788: Test java/lang/Thread/virtual/ThreadAPI.testGetStackTrace3 fails
  • 8e602b8: 8286783: Expand use of @inheritdoc in InputStream and OutputStream subclasses
  • ea713c3: 8286810: Use public [Double|Float].PRECISION fields in jdk.internal.math.[Double|Float]Consts
  • 87d9d7f: 8286596: AArch64: -XX:UseBranchProtection=pac-ret crashes after JDK-8284161
  • af07919: 8286729: G1: Calculation to fit in optional region in remaining pause time wrong
  • c0d51d4: 8282080: Lambda deserialization fails for Object method references on interfaces
  • ... and 166 more: https://git.openjdk.java.net/jdk/compare/080f3c5d8a2f7b2d13baf98c594d4ace67608fc4...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated label May 17, 2022
@openjdk openjdk bot closed this May 17, 2022
@openjdk openjdk bot removed ready rfr labels May 17, 2022
@jonathan-gibbons jonathan-gibbons deleted the 8286338.author-warnings branch May 17, 2022
@openjdk
Copy link

openjdk bot commented May 17, 2022

@jonathan-gibbons Pushed as commit 141ef68.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated javadoc
2 participants