[9] Code Review Request For 8163384: Fix doclint errors and warnings in javafx.graphics module

Chien Yang chien.yang at oracle.com
Wed Mar 1 16:27:51 UTC 2017


Hi Jonathan,

Thanks for the valuable recommendations! I will make corrections based 
on this list. Please send me additional recommendations if you have more.

Thanks,
- Chien

On 2/28/17, 4:00 PM, Jonathan Gibbons wrote:
> Chien,
>
> It's good to see these issues being addressed. I've browsed maybe a 
> third of the files, enough to see some common patterns that you might 
> want to address. The most prevalent comment is issues with 
> combinations of <pre>, {@code}, {@literal}, and entities, but I'm also 
> seeing empty descriptions for methods, and empty alt text for images 
> as well.
>
> Providing empty alt="" for images is not really living up to the 
> intent of the Accessibility Guidelines.
> See https://www.w3.org/TR/WCAG20-TECHS/H37.html
>
> There are some places, e.g.
> http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/modules/javafx.graphics/src/main/java/javafx/concurrent/Service.java.sdiff.html 
>
> where it might be more convenient to use a multi-line {@code } block, 
> rather than use entities for awkward characters in the code.
> In general, the use of <pre><code>...</code></pre> should be 
> considered an anti-pattern.  Consider using <pre>{@code ... }</pre> 
> instead.
>
> http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/modules/javafx.graphics/src/main/java/javafx/css/CssMetaData.java.sdiff.html 
>
> line 59, use <pre>{@code
>
> http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/modules/javafx.graphics/src/main/java/javafx/css/Size.java.sdiff.html 
>
> Missing method comments. e.g. line 45, 53
>
> http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/modules/javafx.graphics/src/main/java/javafx/css/StyleConverter.java.sdiff.html 
>
> line 66, Don't use entities inside {@code}
>
> http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/modules/javafx.graphics/src/main/java/javafx/css/StyleConverter.java.sdiff.html 
>
> line 186: Weird mix of <code> and {@literal}.  Just use {@code}.
>
> http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/modules/javafx.graphics/src/main/java/javafx/css/StyleConverter.java.sdiff.html 
>
> More methods with no descriptions
>
> http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/modules/javafx.graphics/src/main/java/javafx/css/StyleablePropertyFactory.java.sdiff.html 
>
> Use <pre>{@code
>
> http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/modules/javafx.graphics/src/main/java/javafx/css/StyleablePropertyFactory.java.sdiff.html 
>
> line 550: <pre>{@code} is more idiomatic than <pre>{@literal
>
> http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/modules/javafx.graphics/src/main/java/javafx/css/StyleablePropertyFactory.java.sdiff.html 
>
> There are many instances in the file where a pattern like 
> "CssMetaData<S, Enum>" might be better written with an enclosing 
> {@code } or {@literal.
>
> -- Jon
>
> On 02/28/2017 03:00 PM, Chien Yang wrote:
>> Hi all,
>>
>> Please review the proposed fix.
>>
>> JIRA: https://bugs.openjdk.java.net/browse/JDK-8163384
>>
>> Webrev: http://cr.openjdk.java.net/~ckyang/JDK-8163384/webrev.00/
>>
>>
>> Thanks,
>>
>> - Chien
>>
>


More information about the openjfx-dev mailing list