On Thu, 4 Jan 2024 13:38:31 GMT, Nir Lisker <[email protected]> wrote:
>> John Hendrikx has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains eight additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into feature/selector-performance-improvement
>> - Add since tag to new API
>> - Add deprecated annotation and fixed deprecation descriptions
>> - Use copyOf instead of Collections.unmodifiableList(new ArrayList<>(x))
>> - Pull up frozen field to abstract FixedCapacitySet class
>> - Add copyright header
>> - Deprecate StyleClass and remove StyleClassSet for faster solution
>> - Fix regression
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/FixedCapacitySet.java
> line 81:
>
>> 79: * collection or making a read-only copy.
>> 80: */
>> 81: public abstract void freeze();
>
> All the permitted subclasses have the same implementation of this method. Any
> reason not to pull up the `boolean frozen` field to this class and make this
> method concrete with `frozen = true;`?
I've pulled it up, it does make it a bit nicer.
> modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java line 81:
>
>> 79: * @deprecated for future removal, use {@link #getStyleClassNames()}
>> instead
>> 80: */
>> 81: public List<String> getStyleClasses() {
>
> Deprecating requires the `@Deprecate` annotation (possibly with setting
> `forRemoval = true`). The javadoc tag needs only to mention why (e.g., "no
> longer needed because...") and what to replace it with.
>
> Same for the other deprecations.
Thanks, I've updated these.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1442425021
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1442424866
