search

miho / VWorkflows

Flow Visualization Library for JavaFX and VRL-Studio
http://vworkflows.mihosoft.eu
Other
296 stars 67 forks source link

Add support for jdk11+ #65

Closed willcohen closed 5 years ago

willcohen commented 5 years ago

Addresses #63, by only conditionally adding openjfx when the JDK is detected as 11 or higher.

willcohen commented 5 years ago

As a note for the future, it looks like JDK13+ is incompatible with gradle 5.6, but it looks like once gradle 6 is released that can get added to the testing matrix as well.

miho commented 5 years ago

Thanks for addressing this issue! I will review the PR ASAP.

willcohen commented 5 years ago

Of course! Nothing actually changed besides dependency management so I don't think it's super pressing. Two considerations that you may want me to address in this PR after reviewing but before merging:

1) Since the logic in gradle is conditionally choosing to add openjfx as a dependency depending on the JDK used to build, I suspect this probably means that JDK 8 should still be used to actually compile the artifacts that get deployed (even though JDK 11 will compile to target 1.8, it'll have the extra dependencies in there) so that JDK 11 javafx doesn't get added in every time. This will mean that users of JDK 11+ will still need to add openjfx into their own dependencies, but at least the current testing process will ensure that we know that compatibility is maintained going forward for users of newer JDKs. I'm not sure how else to solve this as long as JDK 8 is supported.

2) I fixed the javadoc dependencies using syntax that worked on JDK 11+, but I notice that errors still persist on JDK 8. None of it matters that much, but it either means that the deployment of javadocs should be done on 11+ even though the artifacts themselves are built on 8, or I should change the syntax of the modified annotations to be compatible with 8.

miho commented 5 years ago

Thanks again. The PR is merged.