On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> - 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:
>
> Fix indentations and merge short lines
1 revert and a follow-up bug is requested.
This is one giant pull request, somewhat difficult on the reviewers, especially
since we have to go through it several times.
It would be *much* easier for the reviewers to deal with one fix per PR,
especially in the cases where more than 30 files are going to be touched.
For example, changes involving diamond operator and missing @overrides could
have been made into two separate PRs, making it a breeze to review and
integrate, while also cutting down on the number of files which contained
non-trivial changes.
What do you think?
modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java
line 601:
> 599: }
> 600: args.add("--" + k + "=" + (v != null ? v : ""));
> 601: idx++;
I suspect there might be a deviation from original intent that had happened
earlier. The original idea seems to be of capturing IOException and sending
some diagnostic output to stderr, then continuing. I am guessing here, but
perhaps at some point in the past `Base64.getDecoder().decode()` replaced
whatever was there before - and instead of throwing `IOException` it now throws
`IllegalArgumentException` if things cannot be parsed.
So the try-catch block should remain, catching an `IllegalArgumentException`
instead.
Perhaps we ought to revert these changes and file a follow-up bug?
What do you think?
modules/javafx.graphics/src/main/java/com/sun/javafx/css/PseudoClassState.java
line 172:
> 170:
> 171: static final List<PseudoClass> pseudoClasses =
> 172: new ArrayList<>();
minor: if you make further changes, these two can be on one line.
modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java
line 586:
> 584: ascent = -(float)hhea.getShort(4);
> 585: descent = -(float)hhea.getShort(6);
> 586: linegap = hhea.getShort(8);
interesting: why not on the previous 2 lines? isn't
`-(float)shortValue == (float)(-shortValue)` ?
modules/javafx.graphics/src/main/java/com/sun/prism/d3d/D3DContext.java line
514:
> 512: case D3DERR_DEVICEREMOVED:
> 513: return "D3DERR_DEVICEREMOVED";
> 514: case D3D_OK:
this change is probably correct, but one can think of a theoretical possibility
of collision, i.e.
`hResult = ((1L << 32) + D3DERR_DEVICENOTRESET)` will hit
`D3DERR_DEVICENOTRESET` case instead of the default one.
but again, hResult is long perhaps because it's originally unsigned32 or
something like that.
modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 2579:
> 2577: term = nextLayer(lastTerm);
> 2578: }
> 2579: return new ParsedValueImpl<>(layers,
> CornerRadiiConverter.getInstance());
My eclipse has a problem with this class (might be an Eclipse bug). It does
compile, but any time I modify the class it generates a bunch of errors. The
only code that does not produce the errors is returning a raw value.
-------------
Changes requested by angorya (Committer).
PR: https://git.openjdk.org/jfx/pull/960
