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/