On Wed, 28 Feb 2024 21:48:38 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> This change brings the number of javadoc warnings back to 91 (to be fixed in > [JDK-8270996](https://bugs.openjdk.org/browse/JDK-8270996)) > > - adds missing information in `@param` tags > - adds `@SuppressWarnings("doclint:missing")` to `Skinnable` to silence the > warning due to [JDK-8325071](https://bugs.openjdk.org/browse/JDK-8325071) > - fixed an empty `<p>` in `Subscription` > - cleaned up unnecessary `@throws` in Filtered/SortedList > > Q: Does this PR need a CSR? I did a quick pass, and confirm that there is nothing here that needs a CSR. There are enough changes that a second pair of eyes might be helpful, but not required. I left a few inline comments. I might have more when I do a second pass. modules/javafx.base/src/main/java/javafx/beans/binding/When.java line 810: > 808: * An intermediate class needed while assembling the ternary > expression. It > 809: * should not be used in another context. > 810: * @param <T> the type of an object being built Suggestion: type of "the" object modules/javafx.base/src/main/java/javafx/collections/transformation/FilteredList.java line 164: > 162: * @param index index of the element to return > 163: * @return the element at the specified position in this list > 164: * @throws IndexOutOfBoundsException {@inheritDoc} Was this generating a warning? Does the superclass specify `@throws`? modules/javafx.base/src/main/java/javafx/collections/transformation/SortedList.java line 163: > 161: * @param index index of the element to return > 162: * @return the element at the specified position in this list > 163: * @throws IndexOutOfBoundsException {@inheritDoc} Same question as above about the removal of the `@throws` modules/javafx.base/src/main/java/javafx/util/Builder.java line 32: > 30: * construct other objects. > 31: * > 32: * @param <T> the type of an object being built Suggestion: type of "the" object. modules/javafx.base/src/main/java/javafx/util/Pair.java line 32: > 30: > 31: /** > 32: * <p>A convenience class to represent name-value pairs.</p> Maybe also remove the redundant `<p>` and `</p>` tags? modules/javafx.base/src/main/java/javafx/util/StringConverter.java line 32: > 30: * The type of objects and formats of strings are defined by the > subclasses > 31: * of Converter. > 32: * @param <T> the type of an object being converted Suggestion: type of "the" object. modules/javafx.base/src/main/java/javafx/util/converter/FormatStringConverter.java line 36: > 34: * instance.</p> > 35: * > 36: * @param <T> the type of an object being converted Suggestion: type of "the" object. modules/javafx.controls/src/main/java/javafx/scene/control/ScrollToEvent.java line 37: > 35: * {@link ListView}, {@link TableView}, {@link TreeView} and {@link > TreeTableView}. > 36: * > 37: * @param <T> the scroll to event type Minor: Maybe hyphenate "scroll-to"? ------------- PR Review: https://git.openjdk.org/jfx/pull/1384#pullrequestreview-1907683236 PR Review Comment: https://git.openjdk.org/jfx/pull/1384#discussion_r1506825695 PR Review Comment: https://git.openjdk.org/jfx/pull/1384#discussion_r1506822986 PR Review Comment: https://git.openjdk.org/jfx/pull/1384#discussion_r1506823180 PR Review Comment: https://git.openjdk.org/jfx/pull/1384#discussion_r1506826213 PR Review Comment: https://git.openjdk.org/jfx/pull/1384#discussion_r1506823672 PR Review Comment: https://git.openjdk.org/jfx/pull/1384#discussion_r1506825090 PR Review Comment: https://git.openjdk.org/jfx/pull/1384#discussion_r1506826533 PR Review Comment: https://git.openjdk.org/jfx/pull/1384#discussion_r1506827331
