Re: RFR: 8216497: javadoc should auto-link to platform classes [v2]
On Tue, 22 Sep 2020 17:30:19 GMT, Jonathan Gibbons wrote:
>> Hannes Wallnöfer has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Merge pull request #1 from lahodaj/JDK-8216497
>>
>>Automatically generate the elements-list data from the ct.sym for
>> releases 11+, including the current release under
>>development
>> - Generating current release list for javadoc; using hardcoded lists for
>> versions < 11
>> - Attempting to (mostly) generate the javadoc release manifests from ct.sym
>> historical data.
>
> test/langtools/jdk/javadoc/doclet/testAnnotationTypes/TestAnnotationTypes.java
> line 49:
>
>> 47: javadoc("-d", "out-1",
>> 48: "-sourcepath", testSrc,
>> 49: "--no-platform-link",
>
> I see lots of instances of `no-platform-link` in this and subsequent tests.
> `JavadocTester` does have the concept of
> default options, although that may be more for the behavior after executing
> javadoc and not for the options given to
> javadoc itself. Is it worth supporting default javadoc options, since that
> the default can be disabled for specific
> tests?
I can't really find how `JavadocTester` uses or supports default options. My
concern with this would be that it would
make JavadocTester less transparent and intuitive to use, as you'd have to be
aware what the default options are.
-
PR: https://git.openjdk.java.net/jdk/pull/171
Re: RFR: 8216497: javadoc should auto-link to platform classes [v2]
On Mon, 21 Sep 2020 10:47:40 GMT, Hannes Wallnöfer wrote:
>> This pull request is identical with the RFR previously sent for the
>> Mercurial repository:
>>
>> https://mail.openjdk.java.net/pipermail/javadoc-dev/2020-August/001796.html
>>
>> I'm copy-pasting the comments from the original RFR below.
>>
>> Most of the new code is added to the Extern class where it fits in quite
>> nicely and can use the existing supporting
>> code for setting up external links.
>> The default behaviour is to generate links to docs.oracle.com for released
>> versions and download.java.net for
>> prereleases. Platform documentation URLs can be configured using the new
>> --link-platform-properties option to
>> provide a properties file with URLs pointing to to alternative locations.
>> See the CSR linked above for more details on
>> the new options. The feature can be disabled using the --no-platform-link
>> option, generating the same output as
>> previously. One problem I had to solve was how to handle the transition
>> from prerelease versions to final releases,
>> since the location of the documentation changes in this transition. For
>> obvious reasons we don’t want to make that
>> switch via code change at a time shortly before release. The way it is done
>> is that we determine if the current
>> javadoc instance is a prerelease version as indicated by the Version
>> returned by BaseConfiguration#getDocletVersion(),
>> and then check whether the release/source version of the current javadoc
>> execution uses the same (latest) version. This
>> means that that only the latest version will ever generate prerelease links
>> (e.g. running current javadoc 16 with
>> source version 15 will generate links to the final release documentation)
>> but I think this is acceptable. Another
>> issue I spent some time getting right was tests. New releases require a new
>> element-list resource*), so tests have to
>> pick up new releases. On the other hand, we don’t want hundreds of tests to
>> fail when a new release is added, ideally
>> there should be one test with a clear message. Because of this, when a
>> release is encountered for which no
>> element-list is available a warning is generated instead of an error, and
>> the documentation is generated without
>> platform links as if running with --no-platform-link option. This allows
>> most existing tests to pass and just the new
>> test to fail with a relatively clear message of what is wrong.
>> *) I also thought about generating the element-list for the current release
>> at build time. It’s quite involved, and we
>> still need to maintain element-lists for older versions, so I’m not sure
>> it’s worth it.
>>
>> For existing tests that check output affected by the new option I added the
>> --no-platform-link option to disable the
>> feature. Otherwise we’d have to update those tests with each new release (or
>> else freeze the tests to use some static
>> release or source version, which we don’t want either). I updated the CSR
>> to the new code. It also needs to be
>> reviewed: https://bugs.openjdk.java.net/browse/JDK-8251181
>>
>> Thanks,
>> Hannes
>
> Hannes Wallnöfer has updated the pull request incrementally with three
> additional commits since the last revision:
>
> - Merge pull request #1 from lahodaj/JDK-8216497
>
>Automatically generate the elements-list data from the ct.sym for releases
> 11+, including the current release under
>development
> - Generating current release list for javadoc; using hardcoded lists for
> versions < 11
> - Attempting to (mostly) generate the javadoc release manifests from ct.sym
> historical data.
Generally excellent. Some feedback inline.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
line 350:
> 348:
> 349: doclet.usage.excludedocfilessubdir.parameters=\
> 350: :..
3 dots for ellipsis? 2 dots is "parent directory"
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
line 384:
> 382:
> 383: doclet.usage.no-platform-link.description=\
> 384: Do not generate links to platform documentation
Suggest:
Do not generate links to the platform documentation
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java
line 194:
> 192:
> 193: /**
> 194: * Argument for command-line option {@code --no-platform-link}.
minor: would "--no-platform-links" (plural) be a better name for the option?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseOptions.java
line 435:
> 433: },
> 434:
> 435: new Option(resources, "--no-platform-link") {
Repeating preceding comment: would `--no-platform-links` (plural) be a better
name?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java
line 236:
> 234: * @param linkPlatformProperties path or URL to properties
Re: RFR: 8216497: javadoc should auto-link to platform classes [v2]
On Tue, 22 Sep 2020 17:24:19 GMT, Jonathan Gibbons wrote:
>> Hannes Wallnöfer has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Merge pull request #1 from lahodaj/JDK-8216497
>>
>>Automatically generate the elements-list data from the ct.sym for
>> releases 11+, including the current release under
>>development
>> - Generating current release list for javadoc; using hardcoded lists for
>> versions < 11
>> - Attempting to (mostly) generate the javadoc release manifests from ct.sym
>> historical data.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java
> line 323:
>
>> 321: props.load(inputStream);
>> 322: }
>> 323: url = props.getProperty("doclet.platform.docs." + version);
>
> Similar to other file-or-url arguments: good!
As a possibly-later cleanup, should we have a single utility method somewhere
(in this class) to open a stream on a
file-or-url?
-
PR: https://git.openjdk.java.net/jdk/pull/171
Re: RFR: 8216497: javadoc should auto-link to platform classes [v2]
> This pull request is identical with the RFR previously sent for the Mercurial > repository: > > https://mail.openjdk.java.net/pipermail/javadoc-dev/2020-August/001796.html > > I'm copy-pasting the comments from the original RFR below. > > Most of the new code is added to the Extern class where it fits in quite > nicely and can use the existing supporting > code for setting up external links. > The default behaviour is to generate links to docs.oracle.com for released > versions and download.java.net for > prereleases. Platform documentation URLs can be configured using the new > --link-platform-properties option to > provide a properties file with URLs pointing to to alternative locations. See > the CSR linked above for more details on > the new options. The feature can be disabled using the --no-platform-link > option, generating the same output as > previously. One problem I had to solve was how to handle the transition > from prerelease versions to final releases, > since the location of the documentation changes in this transition. For > obvious reasons we don’t want to make that > switch via code change at a time shortly before release. The way it is done > is that we determine if the current > javadoc instance is a prerelease version as indicated by the Version returned > by BaseConfiguration#getDocletVersion(), > and then check whether the release/source version of the current javadoc > execution uses the same (latest) version. This > means that that only the latest version will ever generate prerelease links > (e.g. running current javadoc 16 with > source version 15 will generate links to the final release documentation) but > I think this is acceptable. Another > issue I spent some time getting right was tests. New releases require a new > element-list resource*), so tests have to > pick up new releases. On the other hand, we don’t want hundreds of tests to > fail when a new release is added, ideally > there should be one test with a clear message. Because of this, when a > release is encountered for which no > element-list is available a warning is generated instead of an error, and the > documentation is generated without > platform links as if running with --no-platform-link option. This allows most > existing tests to pass and just the new > test to fail with a relatively clear message of what is wrong. > *) I also thought about generating the element-list for the current release > at build time. It’s quite involved, and we > still need to maintain element-lists for older versions, so I’m not sure > it’s worth it. > > For existing tests that check output affected by the new option I added the > --no-platform-link option to disable the > feature. Otherwise we’d have to update those tests with each new release (or > else freeze the tests to use some static > release or source version, which we don’t want either). I updated the CSR to > the new code. It also needs to be > reviewed: https://bugs.openjdk.java.net/browse/JDK-8251181 > > Thanks, > Hannes Hannes Wallnöfer has updated the pull request incrementally with three additional commits since the last revision: - Merge pull request #1 from lahodaj/JDK-8216497 Automatically generate the elements-list data from the ct.sym for releases 11+, including the current release under development - Generating current release list for javadoc; using hardcoded lists for versions < 11 - Attempting to (mostly) generate the javadoc release manifests from ct.sym historical data. - Changes: - all: https://git.openjdk.java.net/jdk/pull/171/files - new: https://git.openjdk.java.net/jdk/pull/171/files/2aed84f8..6d659ae3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=171=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=171=00-01 Stats: 2007 lines in 9 files changed: 308 ins; 1698 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/171.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/171/head:pull/171 PR: https://git.openjdk.java.net/jdk/pull/171
