On Tue, 29 Nov 2022 00:25:40 GMT, Andy Goryachev <ango...@openjdk.org> 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 for the utility method, I think the solution is adequate for now and there's
really not much more to gain. Even before the changes (where I just remove
`StubToolkit` casts) all tests passed, so I have to wonder what exactly the
point is of these checks.
As far as I can see, the tests should be run with the system property
`javafx.toolkit` set to `test.com.sun.javafx.pgstub.StubToolkit`. Without it,
many tests fail (probably thousands). Verifying this in each test that happens
to require `StubToolkit` seems overkill -- it could be a single test in the
root somewhere that checks this (or not at all). There is even special test
code in the **actual** `Toolkit` class:
boolean printToolkit = verbose
|| (userSpecifiedToolkit &&
!forcedToolkit.endsWith("StubToolkit"));
This basically is done to avoid printing out the Toolkit name to stderr; why
this is so important to not to do when it is a `StubToolkit` (which is only
used during testing) is something I can't explain.
> but these are not show stoppers, I'll leave it to your discretion.
>
> And finally, once the cleanup PRs are in, I'd like you to publish the
> **changes** to the Eclipse warnings configuration, i.e. let us know which
> warnings should be enabled.
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:
[javafx.epf.txt](https://github.com/openjdk/jfx/files/10110775/javafx.epf.txt)
> modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line
> 799:
>
>> 797: private static class StyleableProperties {
>> 798: private static final CssMetaData<Control,String> SKIN =
>> 799: new CssMetaData<>("-fx-skin",
>
> one line?
Fixed
-------------
PR: https://git.openjdk.org/jfx/pull/959
