On Thu, 15 Oct 2020 00:31:51 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
> The overall pages is very dynamic and depends on a lot of factors, such as
> the input files and the command-line
> options. Some of the summary files depend on the content of the classes being
> documented, such as:
> * any constant values declared in the files
> * any deprecated elements
> * any serializable classes
> * any classes whose description references system properties
>
> This change is a step towards handling these files in a uniform manner.
>
> There is a new test to test the consistency of the new behavior..
Looks good, Jon. The inline comments all refer to minor issues.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HelpWriter.java
line 276:
> 274: if
> (configuration.conditionalPages.contains(HtmlConfiguration.PageKind.SYSTEM_PROPERTIES))
> {
> 275: section = newHelpSection(contents.systemPropertiesLabel);
> 276: Content constantsBody =
> getContent("doclet.help.systemProperties.body",
Not an actual problem, but `constantsBody` is the name used for the constant
values page. Should be either a generic
name or one hinting at the system properties content.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java
line 136:
> 134: // Note: this should (eventually) be merged with Navigation.PageMode,
> 135: // which performs a somewhat similar role
> 136: public enum PageKind {
Maybe call it `ConditionalPage` and change comment to "Kinds of conditional
pages" to make purpose/scope clearer?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DeprecatedListWriter.java
line 267:
> 265: public static void generate(HtmlConfiguration configuration) throws
> DocFileIOException {
> 266: DeprecatedAPIListBuilder deprAPI =
> configuration.deprecatedAPIListBuilder;
> 267: if (deprAPI != null && !deprAPI.isEmpty()) {
Just a note regarding consistency: everywhere else you are using
`configuration.conditionalPages.contains(HtmlConfiguration.PageKind.DEPRECATED)`
for this check.
-------------
Marked as reviewed by hannesw (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/674
