JDK-8274639: Provide a way to disable warnings for cross-modular links #5900

hns · 2021-10-11T19:12:00Z

This is a simple change to add a new --link-modularity-mismatch (warn|info) command line option to allow messages about wrong modularity of externally linked documentation bundles to be issued as warnings or notices.

CSR: https://bugs.openjdk.java.net/browse/JDK-8274969

Progress

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

Issues

JDK-8274639: Provide a way to disable warnings for cross-modular links
JDK-8274969: Javadoc option to disable warnings for cross-modular links (CSR) ⚠️ Issue is not open.

Reviewers

Jonathan Gibbons (@jonathan-gibbons - Reviewer) ⚠️ Review applies to db81fc4

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5900

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

Using diff file

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

bridgekeeper · 2021-10-11T19:12:45Z

👋 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 · 2021-10-11T19:15:55Z

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

mlbridge · 2021-10-11T19:19:19Z

Webrevs

hns · 2021-10-14T10:27:28Z

/csr

openjdk · 2021-10-14T10:28:10Z

@hns this pull request will not be integrated until the CSR request JDK-8274969 for issue JDK-8274639 has been approved.

jonathan-gibbons

It's OK, but I think it would be better off using a enum rather than a boolean to control the diagnostic generation.

...avadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties

jonathan-gibbons · 2021-11-08T19:21:21Z

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java

+     * Argument for command-line option {@code --link-modularity-mismatch}.
+     * True if warnings on external documentation with non-matching modularity should be omitted.
+     */
+    private boolean linkModularityNoWarning = false;


Would it be better to use a local enum that matches the option values?

I introduced an enum, I agree it's an improvement. Hope you are ok with the naming.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java

jonathan-gibbons

Minor naming suggestions, but generally approved.

jonathan-gibbons · 2021-12-01T23:00:13Z

...avadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties

+    Report external documentation with wrong modularity as either\n\
+    warning or informational message. The default behaviour is to\n\


suggest:

Report external documentation with wrong modularity with either\n
a warning or informational message.

(change "as" to "with"; add "a" before "warning")

jonathan-gibbons · 2021-12-01T23:01:03Z

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java

+    public enum ModularityMismatchPolicy {
+        info,
+        warn
+    }


enum name is good; constants could be upper case?

jonathan-gibbons · 2021-12-01T23:02:07Z

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java

+                    public boolean process(String opt, List<String> args) {
+                        String s = args.get(0);
+                        switch (s) {
+                            case "warn", "info" -> linkModularityMismatch = ModularityMismatchPolicy.valueOf(s);


If the constants were upper case, you could use s.toUpperCase(Locale.ROOT)

openjdk · 2021-12-01T23:06:24Z

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

8274639: Provide a way to disable warnings for cross-modular links

Reviewed-by: jjg

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

088b244: 8251216: Implement MD5 intrinsics on AArch64
a093cdd: 8276657: XSLT compiler tries to define a class with empty name
8198807: 8266435: WBMPImageReader.read() should not truncate the input stream
b226ab9: 8276141: XPathFactory set/getProperty method
09522db: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures
67745fa: 8276299: G1: Unify the wording buffer/node/element in G1SegmentedArrayXxx, G1CardSetXxx and related classes
51d6d7a: 8266839: Enable pandoc on macosx-aarch64 at Oracle
0dfb3a7: 8268582: javadoc throws NPE with --ignore-source-errors option
f41e768: 8277762: Allow configuration of HOTSPOT_BUILD_USER
a363b7b: 8177819: DateTimeFormatterBuilder zone parsing should recognise DST
... and 772 more: https://git.openjdk.java.net/jdk/compare/c05dc268acaf87236f30cf700ea3ac778e3b20e5...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.

hns · 2021-12-02T09:38:55Z

/integrate

openjdk · 2021-12-02T09:40:35Z

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

088b244: 8251216: Implement MD5 intrinsics on AArch64
a093cdd: 8276657: XSLT compiler tries to define a class with empty name
8198807: 8266435: WBMPImageReader.read() should not truncate the input stream
b226ab9: 8276141: XPathFactory set/getProperty method
09522db: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures
67745fa: 8276299: G1: Unify the wording buffer/node/element in G1SegmentedArrayXxx, G1CardSetXxx and related classes
51d6d7a: 8266839: Enable pandoc on macosx-aarch64 at Oracle
0dfb3a7: 8268582: javadoc throws NPE with --ignore-source-errors option
f41e768: 8277762: Allow configuration of HOTSPOT_BUILD_USER
a363b7b: 8177819: DateTimeFormatterBuilder zone parsing should recognise DST
... and 772 more: https://git.openjdk.java.net/jdk/compare/c05dc268acaf87236f30cf700ea3ac778e3b20e5...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-12-02T09:40:52Z

@hns Pushed as commit 103da8f.

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

hns added 2 commits October 4, 2021 11:11

JDK-8274639: Avoid warnings when creating cross-modular links

54c1ccf

JDK-8274639: Provide a way to disable warnings for cross-modular links

1a3cce0

openjdk bot added the rfr Pull request is ready for review label Oct 11, 2021

openjdk bot added the javadoc javadoc-dev@openjdk.org label Oct 11, 2021

openjdk bot added the csr Pull request needs approved CSR before integration label Oct 14, 2021

jonathan-gibbons reviewed Nov 8, 2021

View reviewed changes

hns added 2 commits November 10, 2021 16:21

Review feedback

3ae2a9c

Improve l10n hint

db81fc4

openjdk bot removed the csr Pull request needs approved CSR before integration label Nov 10, 2021

jonathan-gibbons approved these changes Dec 1, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Dec 1, 2021

More review feedback

cddace7

openjdk bot closed this Dec 2, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 2, 2021

		Report external documentation with wrong modularity as either\n\
		warning or informational message. The default behaviour is to\n\

JDK-8274639: Provide a way to disable warnings for cross-modular links #5900

JDK-8274639: Provide a way to disable warnings for cross-modular links #5900

Uh oh!

Conversation

hns commented Oct 11, 2021 • 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 Oct 11, 2021

Uh oh!

openjdk bot commented Oct 11, 2021

Uh oh!

mlbridge bot commented Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

hns commented Oct 14, 2021

Uh oh!

openjdk bot commented Oct 14, 2021

Uh oh!

jonathan-gibbons left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonathan-gibbons Nov 8, 2021

Choose a reason for hiding this comment

Uh oh!

hns Nov 10, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonathan-gibbons left a comment

Choose a reason for hiding this comment

Uh oh!

jonathan-gibbons Dec 1, 2021

Choose a reason for hiding this comment

Uh oh!

jonathan-gibbons Dec 1, 2021

Choose a reason for hiding this comment

Uh oh!

jonathan-gibbons Dec 1, 2021

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Dec 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hns commented Dec 2, 2021

Uh oh!

openjdk bot commented Dec 2, 2021

Uh oh!

openjdk bot commented Dec 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

hns commented Oct 11, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Oct 11, 2021 •

edited

Loading

openjdk bot commented Dec 1, 2021 •

edited

Loading