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

John Hendrikx jhendrikx at openjdk.org
Tue Nov 29 08:00:21 UTC 2022


On Tue, 29 Nov 2022 00:25:40 GMT, Andy Goryachev <angorya at 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


More information about the openjfx-dev mailing list