-
Notifications
You must be signed in to change notification settings - Fork 541
8333146: Move maven publication logic from build.gradle #1970
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
base: master
Are you sure you want to change the base?
Conversation
|
👋 Welcome back mhanl! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
Webrevs
|
|
Reviewers: @johanvos @tiainen and one of @kevinrushforth or @arapte /reviewers 2 reviewers |
|
@kevinrushforth |
|
Note that there's a relevant stale PR in #1232 by @jjohannes. Maybe he'd like to take a look. |
Thanks. It looks like my work will not block/interfere the implementation of that PR, which is good. |
|
Thanks for the ping. I have to rework my PR anyway. There is no need to pay special attention to it. Any change that makes the publishing setup clearer is good IMO. I think this PR is a good step forward. (I didn't get back to the topic yet on the my PR, because it is more work as I initially expected. What is complicating things me there is how conceptually get the setup right with the Jars for different platform built on different machines. I have to set aside some mote time for it.) |
|
I found a way to further decouple the maven publishing logic! Now we only need to: It will work like before, so there is no logical change. But still need a re-review. I retested all configurations. |
This PR splits the maven publishing logic into an own file,
maven-publish.gradle, that is next to the rootbuild.gradle.The
build.gradlewill apply themaven-publish.gradle. Themaven-publish.gradlewill then configure the Maven related properties and register all modules for publication.This way, we decoupled the logic that much, the only things we need to do:
maven-publish.gradleconfigureMavenPublicationlater in the build chain.To better understand the context, this is the commit where the whole Maven Publishing logic was introduced. I moved all of that logic out of the main
build.gradle: 5a18677This will reduce the size by ~170 lines for the
build.gradle.Tested with:
./gradlew -PMAVEN_PUBLISH=true -PMAVEN_VERSION=custom publishToMavenLocal./gradlew -PMAVEN_PUBLISH=true publishToMavenLocal./gradlew -PMAVEN_PUBLISH=true -PMILESTONE_FCS=true publishToMavenLocalEverything still works:

-> Example: javafx.base from the local .m2 repository
I think this is a good step and an easy way to split out functionality without blowing things up. We might want to do that for other parts as well.
Note: I also fixed the deprecated
buildDir, the deprecatedproject.taskmethod and 2 warnings where it seems like he might not be able to infer the type (changingdefto the actual type).-> The file is completely green, no warnings or deprecations.
Note2: I did a small improvement to the
addMavenPublicationmethod. It is now more 'typesafe', e.g. projects must be passed in directly, not as String. If a project does not exist, the build will fail with an exception.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1970/head:pull/1970$ git checkout pull/1970Update a local copy of the PR:
$ git checkout pull/1970$ git pull https://git.openjdk.org/jfx.git pull/1970/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1970View PR using the GUI difftool:
$ git pr show -t 1970Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1970.diff
Using Webrev
Link to Webrev Comment