On Thu, 14 Oct 2021 08:39:01 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:
> Please review this simple change to add doc comments to the so far
> undocumented `HtmlStyle` constants. I also removed one more unused constant
> (memberNameLabel).
Approved, but ....
The PR is about providing comments, and (with one typo) they're OK. But it
does seem like some names could be moved into specific existing or new sections
other than just "Miscellaneous". But, I guess I'm OK if you would prefer to do
that as a separate round of cleanup.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 836:
> 834:
> 835: /**
> 836: * The class af a {@code div} element containing a text block that
> is part of a
`s/af/of/`
`text block` sounds too much like the Java construct. Is there a better way to
phrase this? Do you need the word "text" at all?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 842:
> 840:
> 841: /**
> 842: * The class of a {@code ul} element containing text blocks of
> documentation
ditto "text block"
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 860:
> 858: * The class of an {@code a} element for a link with an external
> target.
> 859: */
> 860: externalLink,
I continue to be dubious that this class is worthwhile, since it is not applied
consistently across all docs.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 871:
> 869: * The class of a {@code ul} element with horizontal (inline)
> display style.
> 870: */
> 871: horizontal,
Arguable the word is too short. `horizontal-list` would maybe be better.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 877:
> 875: * a "provides" entry in a module page.
> 876: */
> 877: implementationLabel,
The presence of `Label` would make me expect this is just for the label. If
that's not the case, the name should be changed.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 899:
> 897: * The class of a {@code p} element containing legal copy in the
> page footer.
> 898: */
> 899: legalCopy,
This is another dubious name. It's only a JDK convention that the footer
contains legal info. Either this style should be just `footer` or `legalCopy`
should be defined in a JDK-specific file.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 904:
> 902: * The class of an {@code a} element for a link in member summary
> lists.
> 903: */
> 904: memberNameLink,
If we have/had a section on links, this would belong there.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 910:
> 908: * the serialized form page.
> 909: */
> 910: nameValue,
If we had a section on styles for the serialization page, this would belong
there.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 934:
> 932: * form page.
> 933: */
> 934: serializedPackageContainer,
Another entry for a section on the serialization page
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 957:
> 955: * type signature.
> 956: */
> 957: typeNameLabel,
Isn't there a section for signatures?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
line 962:
> 960: * The class of an {@code a} element for a link to a class or
> interface.
> 961: */
> 962: typeNameLink;
Another candidate for the section on links
-------------
Marked as reviewed by jjg (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5944
