On Mon, 9 Dec 2019 20:58:46 GMT, Hadzic Samir <[email protected]> wrote:
>> Allright, this is a fix for JDK-8207957
>
> The pull request has been updated with 1 additional commit.
The fix looks good to me.
I left a few minor comments, including adding a missing comma in the API docs,
which will also need to be changed in the CSR. Once this is updated I'll review
the CSR and then you can move the CSR to Finalize.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
line 40:
> 39: import javafx.collections.WeakListChangeListener;
> 40: import javafx.css.*;
> 41: import javafx.css.converter.SizeConverter;
Please revert this change. In general, we do not use wildcard imports.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
line 605:
> 604: * custom implementation (such as ones that exclude the header,
> exclude {@code null} content, compute the minimum
> 605: * width etc.).
> 606: *
There should be a `,` before `etc.`
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java
line 2:
> 1: /*
> 2: * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
> 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
add 2019, so:
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java
line 56:
> 55: @Before
> 56: public void before(){
> 57: ObservableList<Person> model = FXCollections.observableArrayList(
Minor: add a space before the `{`
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java
line 80:
> 79: @After
> 80: public void after(){
> 81: sl.dispose();
Minor: add a space before the `{`
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java
line 152:
> 151: tableView.getItems().get(2).setFirstName("Orrin Davies");
> 152: tableView.getItems().get(3).setFirstName("Emma Wilson");
> 153:
It would be nice to not have to repeat the set of strings used. I recommend
putting them in a static string array (they appears either 2 or 3 times in the
test file). Maybe create 4 static strings for the 4 names?
-------------
PR: https://git.openjdk.java.net/jfx/pull/6
