[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