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/0b7ea989...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. I don't think we have a (different) format for annotation arguments, so I've used the function arguments format convention here, no spaces. One point we could use later is to omit TimeUnit and express the value in seconds, but I kept milliseconds for sake of reviewers. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1561#discussion_r1771867083
