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