Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JDK-8220222: specify clearly gradle project dependencies #401

Merged
merged 3 commits into from Apr 3, 2019

Conversation

@christianheilmann
Copy link
Contributor

commented Mar 8, 2019

In order to import the openjdk-jfx project correctly in the different IDEs, the dependencies should be clearly defined. At the moment the project is working in IDEA and Netbeans correctly.

Currently, Netbeans has an issue open with gradle and jigsaw (https://issues.apache.org/jira/browse/NETBEANS-2004 and kelemen/netbeans-gradle-project#417).

So the only IDE working 100% correctly is IDEA for importing the openjdk-jfx project.

https://bugs.openjdk.java.net/browse/JDK-8220222

@@ -1847,12 +1847,24 @@ project(":base") {

sourceSets {
main
shims
test
shims{

This comment has been minimized.

Copy link
@nlisker

nlisker Mar 8, 2019

Missing space before {. Same for all subsequent places.

@kevinrushforth
Copy link
Collaborator

left a comment

I left some specific feedback below, but otherwise looks fine. You will need to fetch the latest upstream changes and merge (not rebase) the upstream develop branch into your branch to fix a trivial merge conflict.

@@ -2396,13 +2431,14 @@ project(":swing") {
//shims // no test shims needed
test
}

This comment has been minimized.

Copy link
@kevinrushforth

kevinrushforth Mar 22, 2019

Collaborator

Please revert this unwanted white-space change.

project.ext.moduleSourcePath = defaultModuleSourcePath
project.ext.moduleSourcePathShim = defaultModuleSourcePathShim

commonModuleSetup(project, [ 'base', 'graphics', 'swing' ])

dependencies {
compile project(":media")

This comment has been minimized.

Copy link
@kevinrushforth

kevinrushforth Mar 22, 2019

Collaborator

javafx.swing does not depend on javafx.media. I think this should instead be:

                compile project(":base")
                compile project(":graphics")
@@ -3119,6 +3179,9 @@ project(":web") {
commonModuleSetup(project, [ 'base', 'graphics', 'controls', 'media', 'web' ])

dependencies {
compile project(":base")
compile project(":graphics")
compile project(":media")

This comment has been minimized.

Copy link
@kevinrushforth

kevinrushforth Mar 22, 2019

Collaborator

Should a dependency on controls be added here as well?

Merge branch 'master' of github.com:javafxports/openjdk-jfx into deve…
…lop and itegrated code review #401

# Conflicts:
#	build.gradle
@christianheilmann

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

@kevinrushforth thanks for your review. You are right in all points and I integrated all your reviews and merged it with the develop branch of upstream.

This checkin enables full working on OpenJFX in IntelliJ IDEA Version 20.18.3 or higher.
Simply open the build.gradle in IntelliJ and you are ready to code.

The checked in IntelliJ project files should be removed, because they do not work and are misleading.
The remove of the IntelliJ project files should be done with #4

@kevinrushforth
Copy link
Collaborator

left a comment

Looks good.

@kevinrushforth kevinrushforth merged commit 4759b5f into javafxports:develop Apr 3, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.