[jfx17] RFR: 8250590: Classes and methods in the javafx.css package are missing documentation [v4]

Kevin Rushforth kcr at openjdk.java.net
Thu Jul 29 12:56:35 UTC 2021


On Thu, 29 Jul 2021 08:43:59 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> This PR corrects/adds missing documentation for classes in javafx.css package.
>
> Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8250590 - fix review comments

I left just a few more (mostly minor) comments.

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 115:

> 113:      *
> 114:      * @param obj an {@code Object} to compare
> 115:      * @return {@code true} if this object is the same as the {@code obj} argument; {@code false} otherwise.

Minor: you can remove the period

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 145:

> 143:     /**
> 144:      * {@inheritDoc}
> 145:      */

Given the `javadoc` tool bug, it's best to revert this (and have no comment on this overridden method).

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 156:

> 154:     /**
> 155:      * {@inheritDoc}
> 156:      */

Revert.

modules/javafx.graphics/src/main/java/javafx/css/Match.java line 86:

> 84:     /**
> 85:      * Gets the specificity.
> 86:      * @return the specificity.

Minor: you can remove the period

modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 102:

> 100:     /**
> 101:      * Gets whether this {@code Selector} applies to the given {@code Styleable}.
> 102:      * It is same as the {@link createMatch} method except it returns true/false rather than a {@code Match}.

It is _the_ same ...

Minor: `true/false` --> `a boolean`?

modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 110:

> 108:     /**
> 109:      * Gets whether this {@code Selector} applies to the given {@code Styleable}.
> 110:      * It is same as {@link applies} method except it also returns

The link happens to work, but since there is more than one `applies` method, it's better to specify which one. Also, it isn't grammatically correct. Maybe something like this:


It is the same as the {@link applies(Styleable) applies(Styleable)} method ...

modules/javafx.graphics/src/main/java/javafx/css/converter/EffectConverter.java line 82:

> 80: 
> 81:     /**
> 82:      * Converter to convert a {@code DropShadow} effect.

Good. Can you also apply this change to `InnerShadowConverter`?

-------------

PR: https://git.openjdk.java.net/jfx/pull/589


More information about the openjfx-dev mailing list