[12] RFR: JDK-8210092: Remove old javafx.swing implementation

Discussion:

Kevin Rushforth

2018-09-21 23:53:25 UTC

Please review the following on GitHub:

https://bugs.openjdk.java.net/browse/JDK-8210092
https://github.com/javafxports/openjdk-jfx/pull/207
https://github.com/javafxports/openjdk-jfx/pull/207/files

This will remove the old JDK-10-based implementation of FX / Swing
interop and cleanup the build to remove the legacy qualified exports and
the logic that optionally excludes the new implementation. It also
changes the dependency that javafx.swing has on jdk.unsupported.desktop
from "requires static" (optional) to "requires", which will fix an
existing bug with jlinked images that include javafx.swing.

-- Kevin

Alan Bateman

2018-09-22 10:03:56 UTC

Permalink

Post by Kevin Rushforth
https://bugs.openjdk.java.net/browse/JDK-8210092
https://github.com/javafxports/openjdk-jfx/pull/207
https://github.com/javafxports/openjdk-jfx/pull/207/files
This will remove the old JDK-10-based implementation of FX / Swing
interop and cleanup the build to remove the legacy qualified exports
and the logic that optionally excludes the new implementation. It also
changes the dependency that javafx.swing has on
jdk.unsupported.desktop from "requires static" (optional) to
"requires", which will fix an existing bug with jlinked images that
include javafx.swing.

I don't know the workflow or how reviews work in the
javafxports/openjdk-jfx project work (I didn't want to hit the "Review
changes" button) but the change to the make the dependency non-optional
looks okay to me.

-Alan.

Prasanta Sadhukhan

2018-09-25 08:41:43 UTC

Permalink

Hi Kevin,

Removal looks ok but one thing, do we still need to keep the
enhanced-for loop in InterOpFactory.java#getInstance now that we have
only 1 factory? I guess we can probably remove the whole reflection
thing also and instantiate InteropFactoryN directly, no?

Regards
Prasanta

Kevin Rushforth

2018-09-25 12:33:27 UTC

Permalink

Yes, that seems like a good bit of additional cleanup. I'll do that.

-- Kevin

Post by Prasanta Sadhukhan
Hi Kevin,
Removal looks ok but one thing, do we still need to keep the
enhanced-for loop in InterOpFactory.java#getInstance now that we have
only 1 factory? I guess we can probably remove the whole reflection
thing also and instantiate InteropFactoryN directly, no?
Regards
Prasanta

Post by Kevin Rushforth
https://bugs.openjdk.java.net/browse/JDK-8210092
https://github.com/javafxports/openjdk-jfx/pull/207
https://github.com/javafxports/openjdk-jfx/pull/207/files
This will remove the old JDK-10-based implementation of FX / Swing
interop and cleanup the build to remove the legacy qualified exports
and the logic that optionally excludes the new implementation. It
also changes the dependency that javafx.swing has on
jdk.unsupported.desktop from "requires static" (optional) to
"requires", which will fix an existing bug with jlinked images that
include javafx.swing.
-- Kevin

Prasanta Sadhukhan

2018-09-27 14:29:48 UTC

Permalink

+1 with the additional refactoring done.

Regards
Prasanta

Post by Kevin Rushforth
Yes, that seems like a good bit of additional cleanup. I'll do that.
-- Kevin

Post by Kevin Rushforth
https://bugs.openjdk.java.net/browse/JDK-8210092
https://github.com/javafxports/openjdk-jfx/pull/207
https://github.com/javafxports/openjdk-jfx/pull/207/files
This will remove the old JDK-10-based implementation of FX / Swing
interop and cleanup the build to remove the legacy qualified exports
and the logic that optionally excludes the new implementation. It
also changes the dependency that javafx.swing has on
jdk.unsupported.desktop from "requires static" (optional) to
"requires", which will fix an existing bug with jlinked images that
include javafx.swing.
-- Kevin