On Wed, 30 Nov 2022 19:08:31 GMT, John Hendrikx <[email protected]> wrote:
>> Andy Goryachev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8294589: cleanup
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java
> line 286:
>
>> 284: ParentHelper.setTraversalEngine(getSkinnable(), engine);
>> 285:
>> 286: lh.addChangeListener(control.sceneProperty(), true, (scene) -> {
>
> minor: generally, the parenthesis are omitted for lambda's with one parameter
> (multiple occurences)
I like consistency:
`() ->` use parentheses
`a ->` don't use, why?
`(a,b) ->` use parentheses
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java
> line 1180:
>
>> 1178:
>> 1179: private static final CssMetaData<MenuBar,Pos> ALIGNMENT =
>> 1180: new CssMetaData<>("-fx-alignment", new
>> EnumConverter<Pos>(Pos.class), Pos.TOP_LEFT ) {
>
> minor: You can use the diamond operator here, probably came from the merge
> conflict
>
> Suggestion:
>
> new CssMetaData<>("-fx-alignment", new
> EnumConverter<>(Pos.class), Pos.TOP_LEFT) {
indeed, thanks!
> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java
> line 227:
>
>> 225: ComboBox.class,
>> 226: DatePicker.class,
>> 227: //MenuBar.class,
>
> minor: commented out code
intentionally, to avoid merge conflicts.
-------------
PR: https://git.openjdk.org/jfx/pull/906
