Hi Jon,

It's nice to see this change addresses a visual regression caused by the recent
changes. Thanks for providing detailed comments for "additional class names"
in the new test.

Nit. Could you change the name of `addExtraCSSClassNames` to 
`addExtraCSSClassNamesTo`
or, better still, rename it and make it create a new immutable set rather than
mutate the passed one.

This change looks good to me.

-Pavel

> On 24 Mar 2020, at 22:46, Jonathan Gibbons <jonathan.gibb...@oracle.com> 
> wrote:
> 
> I forgot to add the bug number into the @bug list in the test. I'll do that 
> in the next webrev (if one is needed) or before pushing.
> 
> -- Jon
> 
> 
> On 3/24/20 3:45 PM, Jonathan Gibbons wrote:
>> Please review some simple fixes to the stylesheet to address some issues 
>> caused by a couple of recent changes: (1) to use "-page" as a suffix for the 
>> CSS class name for <body> elements, and (2) for the change to hyphenated 
>> naming.
>> 
>> A significant new test case is added to the existing TestStylesheet.java 
>> class. The test determines the set of known CSS class names, based on the 
>> CSS class names defined in the stylesheet, and a set of known additional 
>> class names, and then reads the output generated from a small sample API, 
>> and verifies that the values of all the "class" attributes are in set of 
>> known CSS class names.
>> 
>> A copy of the API documentation is provided.  The primary kind of page to 
>> check is a regular type declaration page, such as for java.lang.Object.
>> 
>> -- Jon
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8241544
>> Webrev: http://cr.openjdk.java.net/~jjg/8241544/webrev.00/index.html
>> API: http://cr.openjdk.java.net/~jjg/8241544/api.00/index.html
>> 

Reply via email to