Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v9]

2022-11-29 Thread Nir Lisker
On Tue, 29 Nov 2022 21:58:38 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Nir Lisker
On Tue, 29 Nov 2022 18:37:23 GMT, Kevin Rushforth wrote: >> interesting - there is technically no change, as `TableColumnBase` >> implements `EventTarget`. >> @hjohn does javadoc produce a different result? > > I checked, and yes it does produce a different result: > > Current: > > > public

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v9]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 21:58:38 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v9]

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 21:58:38 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v9]

2022-11-29 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 18:24:29 GMT, Andy Goryachev wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/TableColumn.java >> line 134: >> >>> 132: * @since JavaFX 2.0 >>> 133: */ >>> 134: public class TableColumn extends TableColumnBase { >> >> This class is part of the public

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 16:55:34 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 18:12:26 GMT, Kevin Rushforth wrote: >> John Hendrikx has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 23 commits: >> >> - Merge branch 'master' of https://git.openjdk.org/jfx into >>

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 16:55:34 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-29 Thread John Hendrikx
On Tue, 29 Nov 2022 16:38:26 GMT, Andy Goryachev wrote: > oh no, my PRs are going to wreak havoc with your large PRs. let's make sure > yours get in first. That's okay, merge conflicts can be solved. > > Unsure what you mean by the changes, as I don't know what the default > > setting (or

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 07:41:24 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v6]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 07:56:48 GMT, John Hendrikx wrote: > Unsure what you mean by the changes, as I don't know what the default setting > (or your settings) are, but I've included the settings as an attachment: the warnings being addressed by this PR. - PR:

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 07:41:24 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v6]

2022-11-29 Thread John Hendrikx
On Tue, 29 Nov 2022 00:25:40 GMT, Andy Goryachev wrote: > I think this looks good. My only two suggestions would be > > 1. add util.loadStubToolkit() and > 2. collapse multiple lines where we can I will do so where possible, but a lot of these changes are automated so I may miss some. As

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-29 Thread John Hendrikx
On Mon, 28 Nov 2022 23:32:57 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix some indents and put declarations on one line where possible > >

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-28 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v6]

2022-11-28 Thread Andy Goryachev
On Mon, 28 Nov 2022 21:11:33 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v6]

2022-11-28 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-28 Thread John Hendrikx
On Wed, 23 Nov 2022 23:38:59 GMT, John Hendrikx wrote: >> Yes, but this is now getting to be a bit out of scope for this PR. Even >> adding the `assertTrue` requires running all the tests (which thankfully, >> GHA will do). >> >> Perhaps a better approach is to not make any more changes here,

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v5]

2022-11-28 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-23 Thread Nir Lisker
On Wed, 23 Nov 2022 23:06:45 GMT, John Hendrikx wrote: >> modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/ControlSkinFactory.java >> line 287: >> >>> 285: .map(d -> new Object[] {d, }) >>> 286: .collect(toList()); >>> 287:

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-23 Thread John Hendrikx
On Wed, 23 Nov 2022 23:27:21 GMT, Kevin Rushforth wrote: >> could we just create a method in Util instead of dragging multi-line >> construct across the tests? >> >> e.g. `Util.loadStubToolkit()` ? > > Yes, but this is now getting to be a bit out of scope for this PR. Even > adding the

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-23 Thread Kevin Rushforth
On Wed, 23 Nov 2022 23:21:52 GMT, Andy Goryachev wrote: >> I wouldn't recommend that, since it will then obscure the main reason for >> this call -- to load the toolkit (honestly, the cast was probably an >> afterthought). So this would be OK: >> >> >> tk =

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-23 Thread Andy Goryachev
On Wed, 23 Nov 2022 23:14:52 GMT, Kevin Rushforth wrote: >> I can replace these with an assert: >> >> assertTrue(Toolkit.getToolkit() instanceof StubToolkit); > > I wouldn't recommend that, since it will then obscure the main reason for > this call -- to load the toolkit (honestly, the

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-23 Thread Kevin Rushforth
On Wed, 23 Nov 2022 23:09:55 GMT, John Hendrikx wrote: >>> As for this PR, I don't mind leaving it as is or reverting these changes. I >>> don't think that a CCE is realistic here. >> >> I agree that this isn't a good test design. I was just pointing out why it >> isn't (in theory) a

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-23 Thread Nir Lisker
On Tue, 22 Nov 2022 18:54:39 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-23 Thread John Hendrikx
On Wed, 23 Nov 2022 22:57:07 GMT, Kevin Rushforth wrote: >> This looks like bad test design. If the toolkit needs to be of a specific >> type when running tests, then check that this is the case once before >> running all the tests. As for this PR, I don't mind leaving it as is or >>

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-23 Thread John Hendrikx
On Wed, 23 Nov 2022 22:36:36 GMT, Nir Lisker wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert instanceof changes and replace with null checks > >

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-23 Thread John Hendrikx
On Wed, 23 Nov 2022 22:00:16 GMT, Nir Lisker wrote: >> Yes, good catch, fixed. > > Eclipse's quick fix is to replace the `instanceof` check with a `null` check, > did it not suggest the same for you? I was to quick on the trigger and removed it myself, not Eclipse fault. - PR:

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-23 Thread John Hendrikx
On Wed, 23 Nov 2022 22:48:43 GMT, Kevin Rushforth wrote: >> modules/javafx.controls/src/test/java/test/javafx/scene/chart/CategoryAxisTest.java >> line 122: >> >>> 120: >>> 121: @Test public void checkCategorySpacingReadOnlyCannotBind() { >>> 122:

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-23 Thread Kevin Rushforth
On Wed, 23 Nov 2022 22:50:17 GMT, Nir Lisker wrote: > As for this PR, I don't mind leaving it as is or reverting these changes. I > don't think that a CCE is realistic here. I agree that this isn't a good test design. I was just pointing out why it isn't (in theory) a completely compatible

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-23 Thread Nir Lisker
On Tue, 22 Nov 2022 19:04:00 GMT, Kevin Rushforth wrote: >> Not needed in the setup, if it is `null` later or throws an exception the >> test failure is clear enough. >> >> Also a bit out of scope of this PR. > > Yes, adding an assert does seem out of scope of this PR, although Andy > comment

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-23 Thread Kevin Rushforth
On Wed, 23 Nov 2022 22:40:31 GMT, Nir Lisker wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert instanceof changes and replace with null checks > >

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-23 Thread Kevin Rushforth
On Tue, 22 Nov 2022 18:54:39 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-23 Thread Nir Lisker
On Tue, 22 Nov 2022 18:54:39 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-23 Thread Nir Lisker
On Tue, 22 Nov 2022 18:48:41 GMT, John Hendrikx wrote: >> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/FXVKSkin.java >> line 858: >> >>> 856: protected void sendKeyEvents() { >>> 857: Node target = fxvk.getAttachedNode(); >>> 858: >> >> same

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-22 Thread Kevin Rushforth
On Tue, 22 Nov 2022 18:41:16 GMT, John Hendrikx wrote: >> modules/javafx.controls/src/test/java/test/javafx/scene/control/AccordionTest.java >> line 57: >> >>> 55: >>> 56: @Before public void setup() { >>> 57: tk = Toolkit.getToolkit();//This step is not needed (Just to >>> make

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-22 Thread Kevin Rushforth
On Tue, 22 Nov 2022 18:54:39 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-22 Thread Andy Goryachev
On Tue, 22 Nov 2022 18:54:39 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-22 Thread John Hendrikx
On Tue, 22 Nov 2022 17:38:37 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert "Fix warnings in fxml" >> >> This reverts commit b148aa3cc8a4676167a9eb8a023cddce3de116e7. > >

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v4]

2022-11-22 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-22 Thread John Hendrikx
On Tue, 22 Nov 2022 18:08:19 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert "Fix warnings in fxml" >> >> This reverts commit b148aa3cc8a4676167a9eb8a023cddce3de116e7. > >

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-22 Thread Andy Goryachev
On Tue, 22 Nov 2022 16:44:01 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac`

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v3]

2022-11-22 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v2]

2022-11-22 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be

RFR: JDK-8297414: Remove easy warnings in javafx.controls

2022-11-22 Thread John Hendrikx
Note: I ran into a `javac` compiler bug while replacing types with diamond operators (ecj has no issues). I had two options, add a `SuppressWarnings("unused")` or to use a lambda instead of a method reference to make `javac` happy. I choose the later and added a comment so it can be fixed