On Mon, 11 Oct 2021 19:12:00 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:
> 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 It's OK, but I think it would be better off using a enum rather than a boolean to control the diagnostic generation. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties line 457: > 455: > 456: doclet.usage.link-modularity-mismatch.parameters=\ > 457: (warn|info) This is one of those cases where the words "warn" and "info" should *not* be localized, because they are the values accepted by the option -- assuming that those values are not themselves also localized. I suggest you either add an immediately preceding comment, per the localization guidelines, or else use `({0}|{1})` and provide those strings from the calcite, src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java line 172: > 170: * True if warnings on external documentation with non-matching > modularity should be omitted. > 171: */ > 172: private boolean linkModularityNoWarning = false; Would it be better to use a local enum that matches the option values? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java line 418: > 416: switch (s) { > 417: case "warn" -> linkModularityNoWarning = > false; > 418: case "info" -> linkModularityNoWarning = > true; Maybe use an enum, combine the cases, and use `.valueOf(s)` ------------- PR: https://git.openjdk.java.net/jdk/pull/5900
