On Mon, 28 Nov 2022 21:11:33 GMT, John Hendrikx <jhendr...@openjdk.org> 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` happy. I choose the later and added a comment so
>> it can be fixed once the bug is fixed. I've reported the issue here:
>> https://bugs.openjdk.org/browse/JDK-8297428
>>
>> - Remove unsupported/unnecessary SuppressWarning annotations
>> - Remove reduntant type specifications (use diamond operator)
>> - Remove unused or duplicate imports
>> - Remove unnecessary casts (type is already correct type or can be autoboxed)
>> - Remove unnecessary semi-colons (at end of class definitions, or just
>> repeated ones)
>> - Remove redundant super interfaces (interface that is already inherited)
>> - Remove unused type parameters
>> - Remove declared checked exceptions that are never thrown
>> - Add missing `@Override` annotations
>
> John Hendrikx has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Replace StubToolkit casts with asserts (2)
I think this looks good.
My only two suggestions would be
1. add util.loadStubToolkit() and
2. collapse multiple lines where we can
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.
Thank you.
modules/javafx.controls/src/main/java/javafx/scene/chart/LineChart.java line 86:
> 84: private FadeTransition fadeSymbolTransition = null;
> 85: private Map<Data<X,Y>, Double> XYValueMap =
> 86: new HashMap<>();
can fit on one line now
modules/javafx.controls/src/main/java/javafx/scene/chart/ValueAxis.java line
526:
> 524: private static class StyleableProperties {
> 525: private static final CssMetaData<ValueAxis<? extends
> Number>,Number> MINOR_TICK_LENGTH =
> 526: new CssMetaData<>("-fx-minor-tick-length",
perhaps declarations in this file can fit on two lines now?
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?
modules/javafx.controls/src/test/java/test/javafx/scene/control/SliderTest.java
line 60:
> 58: tk = Toolkit.getToolkit();
> 59:
> 60: assertTrue(tk instanceof StubToolkit); // Ensure it's StubToolkit
I still think that we should create
public static Toolkit Util.loadStubToolkit()?
with a javadoc that explains why it is needed.
-------------
Marked as reviewed by angorya (Author).
PR: https://git.openjdk.org/jfx/pull/959
