Hannes,

Regarding the issues you raised:

  - In PackageUseWriter.java there is a duplicate invocation of getDescription() assigned to an unused variable.
        deleted

  - Not directly related to your patch, but parameter includeScript in HtmlDocletWriter::printHtmlDocument is never used.
        Good catch!
        That took a bit of understanding; it's a leftover from an old refactoring         The functionality that the parameter used to control got moved into HtmlDocletWriter::getBody (which has the includeScript parameter) and the body gets (eventually) passed into printHtmlDocument.  The includeScript parameter for printHtmlDocument is now redundant and should be removed.

  - Line 74 in TestMetadata.java is misaligned.
        Ooops, unexpandex whitespace. Fixed.

Given the minor nature of the changes (deleting a line, fixing whitespace) I've pushed the patch.

As for TestMetadata.java, yes, it's a stronger test than many. The nested loops form what we informally called a "combo-test" in the javac world. The style is good for providing thorough coverage, but it does mean you have to be able to derive the expected behavior from the loop variables, which can be harder in the javadoc world.

Thanks for the review.

-- Jon



On 02/19/2019 07:36 AM, Jonathan Gibbons wrote:
Thanks Hannes,

I'll check out and fix those issues.

-- Jon

On 2/19/19 7:27 AM, Hannes Wallnöfer wrote:
Hi Jon,

Looks good, a few minor issues listed below.

  - In PackageUseWriter.java there is a duplicate invocation of getDescription() assigned to an unused variable.   - Not directly related to your patch, but parameter includeScript in HtmlDocletWriter::printHtmlDocument is never used.
  - Line 74 in TestMetadata.java is misaligned.

Hannes

PS: TestMetadata.java is quite dense, took me some time to review but very instructional.


Am 15.02.2019 um 21:16 schrieb Jonathan Gibbons <[email protected]>:

Please review a medium-small patch to add some new metadata into each HTML document that is generated by javadoc.
The metadata provides the following:

    • a short stylized description of the contents of the file, for potential use by other tools to filter the set of generated files     • an indication of the part of javadoc that generated the file, to help group the different kinds of pages generated by javadoc A new test program is added. It's a bit of overkill, but it is designed to be future-proof, meaning if we add new types of pages, this test will (intentionally) fail until it is updated to know about those new pages.  I also note that the test was instrumental in helping to discover two unrelated bugs in javadoc. These are linked in the "related links" in the JBS issue.

-- Jon
JBS: https://bugs.openjdk.java.net/browse/JDK-8218998
Webrev: http://cr.openjdk.java.net/~jjg/8218998/webrev.00/






Reply via email to