-
Notifications
You must be signed in to change notification settings - Fork 10
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
#350 - Rework publishing plugins so they can be used externally and on multi-project builds #355
base: 7.0.x
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work James! This should hopefully make it easier to publish in the future.
It is hard to test these kind of changes.
Also, I think some projects may depend on it so it will be exciting to see if it is backwards compatible. We'll see 😄.
(I see now that you already mentioned this in your PR description 👍)
src/main/groovy/org/grails/gradle/plugin/publishing/GrailsPublishGradlePlugin.groovy
Outdated
Show resolved
Hide resolved
src/main/groovy/org/grails/gradle/plugin/publishing/GrailsPublishGradlePlugin.groovy
Outdated
Show resolved
Hide resolved
src/main/groovy/org/grails/gradle/plugin/publishing/GrailsPublishGradlePlugin.groovy
Outdated
Show resolved
Hide resolved
src/main/groovy/org/grails/gradle/plugin/publishing/GrailsPublishGradlePlugin.groovy
Outdated
Show resolved
Hide resolved
src/main/groovy/org/grails/gradle/plugin/publishing/GrailsPublishGradlePlugin.groovy
Outdated
Show resolved
Hide resolved
@matrei I believe I've made all of the changes you requested. In terms of backwards compatibility, this will intentionally break the other projects initially - I moved the hardcoded grails repo URL out of the plugin so that it can be used externally. If we define that URL at the organization level, I believe it may "just work", but when I merge this I assumed I'll go through each repo rapidly fixing or converting each repo to a simplified publish. Thank you so much for your feedback! |
.github/workflows/gradle.yml
Outdated
VERSION: ${{ needs.publish.outputs.release_version }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked what it does, and it looks like VERSION
is only needed in release.yml
. So it's OK to remove it here in gradle.yml
.
src/main/groovy/org/grails/gradle/plugin/publishing/GrailsPublishExtension.groovy
Outdated
Show resolved
Hide resolved
src/main/groovy/org/grails/gradle/plugin/publishing/GrailsPublishGradlePlugin.groovy
Outdated
Show resolved
Hide resolved
src/main/groovy/org/grails/gradle/plugin/publishing/GrailsPublishGradlePlugin.groovy
Outdated
Show resolved
Hide resolved
src/main/groovy/org/grails/gradle/plugin/publishing/RepositoryTarget.groovy
Outdated
Show resolved
Hide resolved
developers = [johndoe: 'John Doe'] | ||
} | ||
|
||
By default this plugin will publish to the specified `MAVEN_PUBLISH` instance for snapshots, and `NEXUS_PUBLISH` for releases. To change the snapshot publish behavior, set `snapshotRepoType` to `PublishType.NEXUS_PUBLISH`. To change the release publish behavior, set `releaseRepoType` to `PublishType.NEXUS_PUBLISH`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last sentence, should it not be: To change the release publish behavior, set releaseRepoType
to PublishType.MAVEN_PUBLISH
NEXUS_PUBLISH_USERNAME | ||
NEXUS_PUBLISH_PASSWORD | ||
NEXUS_PUBLISH_STAGING_PROFILE_ID | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same ordering for both types?
NEXUS_PUBLISH_SNAPSHOT_URL | ||
NEXUS_PUBLISH_USERNAME | ||
NEXUS_PUBLISH_PASSWORD | ||
NEXUS_PUBLISH_STAGING_PROFILE_ID | ||
""" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same ordering for both types?
delegate.name title | ||
delegate.description gpe.desc ?: title | ||
delegate.name = title | ||
delegate.description = gpe.desc ?: title |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you can't use = on delegate
. Same applies for every occurrence below. You want to trigger methodMissing
and that will not happen if you call a property setter.
} | ||
boolean mavenPublish = (isSnapshot && snapshotPublishType == PublishType.MAVEN_PUBLISH) || (isRelease && releasePublishType == PublishType.MAVEN_PUBLISH) | ||
boolean sonatypePublish = (isSnapshot && snapshotPublishType == PublishType.NEXUS_PUBLISH) || (isRelease && releasePublishType == PublishType.NEXUS_PUBLISH) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nexusPublish?
This change resolves #350. At a high level it:
grailsPublish
block at the subproject level.With these changes, any project can remove the gradle boiler plate of publishing to maven central & an artifactory snapshot instance. Technically, it doesn't have to be a grails project to use this plugin. If it is a grails plugin then the plugin.xml will be added as an artifact for the publish plugin and for the profile plugin it will generate the associated profile.xml.
I tested the snapshot portion of this logic with this code spring security rest code. @jeffscottbrown was kind enough to assist in testing. You can find an example artifact here - (I published to the wrong repo initially, oops).
Once this change is merged, all of the repositories with
grailsPublish
will have broken publishing since the urls will be missing. I plan to go through all of them quickly and set the appropriate snapshot url (plugins go here & profiles go here) to fix this breakage. Separately, I'll go through the various multi-projects that couldn't use grailsPublish and switch them to grails publish (like the spring security rest plugin).As part of the milestone announcement, we can offer these plugins to people so they can use them to simplify plugin upgrades. I'd like to provide an example github action too.