Pavel,

Thanks, I'll address the "Nit" you raised and follow your suggestions.

-- Jon

On 3/25/20 8:53 AM, Pavel Rappo wrote:
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