On Sun, 22 Sep 2024 13:07:57 GMT, Marius Hanl <mh...@openjdk.org> wrote:

>> Andy Goryachev 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 18 additional 
>> commits since the last revision:
>> 
>>  - review comments
>>  - Merge remote-tracking branch 'origin/master' into 8338468.junit5.controls
>>  - review comments
>>  - Merge remote-tracking branch 'origin/master' into 8338468.junit5.controls
>>  - part 12, 9274 - 185 = 9089
>>  - Merge remote-tracking branch 'origin/master' into 8338468.junit5.controls
>>  - part 11, 9242 tests, 185 ignored
>>  - part 10
>>  - part 9 cell
>>  - part 8
>>  - ... and 8 more: https://git.openjdk.org/jfx/compare/4a43f4fa...55b33b2c
>
> modules/javafx.controls/src/test/java/test/javafx/scene/chart/NumberAxisTest.java
>  line 260:
> 
>> 258: 
>> 259:     @Test
>> 260:     @Timeout(value=1000, unit=TimeUnit.MILLISECONDS)
> 
> minor: I think the convention is to have spaces around the parameters,
> e.g. `@Timeout(value = 1000, unit = TimeUnit.MILLISECONDS)`
> Just scanned the JavaFX code and we seem to be inconsiostent on that, so I 
> will leave that up to do.

@Maran23 is correct about the convention when there is an annotation (or a 
function call, for that matter) with an embedded parameter assignment. For 
example, see various `@Deprecated` annotations in the JDK and JavaFX.

However, as he pointed out, we are not consistent on that, so I would leave it 
as is for this PR to avoid more churn.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1561#discussion_r1771887528

Reply via email to