On Mon, 31 Jul 2023 12:16:05 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> Michael Strauß has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Make TransitionEvent final
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/TransitionDefinitionCssMetaData.java
> line 63:
>
>> 61: private static final Duration[] DURATION_ZERO = new Duration[] {
>> Duration.ZERO };
>> 62:
>> 63: private static final Interpolator[] INTERPOLATOR_EASE = new
>> Interpolator[] { InterpolatorConverter.CSS_EASE };
>
> Careful, the array contents could still be modified
That's true. However, `CssParser` is built around array-based sequences (see
`StringConverter.SequenceConverter`), and changing these arrays to lists would
seem out of place for this part of the codebase. What do you think?
> modules/javafx.graphics/src/main/java/javafx/animation/Interpolator.java line
> 319:
>
>> 317: * The output time value is determined by the {@link StepPosition}.
>> 318: *
>> 319: * @param intervals the number of intervals in the step interpolator
>
> minor: When I see a plural like `intervals` (or `employees`) I think of a
> list of objects. Perhaps `intervalCount` would be better?
Doesn't sound better to me, but I'll defer to what most people feel is best.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1279700471
PR Review Comment: https://git.openjdk.org/jfx/pull/870#discussion_r1279703051
