matrei commented on code in PR #5:
URL:
https://github.com/apache/incubator-grails-gradle-publish/pull/5#discussion_r2259754997
##########
plugin/src/test/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePluginTest.groovy:
##########
@@ -19,24 +19,77 @@
package org.apache.grails.gradle.publish
+import org.gradle.api.GradleException
+import org.gradle.api.internal.project.ProjectInternal
import org.gradle.testfixtures.ProjectBuilder
import spock.lang.Specification
class GrailsPublishGradlePluginTest extends Specification {
- def 'plugin registers release task for release version'() {
+ def "requires java or java platform plugin"() {
given:
def project = ProjectBuilder.builder().build()
project.version = '1.0.0'
when:
project.plugins.apply('org.apache.grails.gradle.grails-publish')
+ ((ProjectInternal) project).evaluate()
then:
- project.tasks.findByName('retrieveSonatypeStagingProfile') != null
+ def ge = thrown(GradleException)
+ ge.cause.message == "Grails Publish Plugin requires the Java Platform
or Java Plugin to be applied to the project."
}
- def 'plugin registers release task for snapshot version'() {
+ def 'apply only: plugin registers release task for release version'() {
+ given:
+ def project = ProjectBuilder.builder().build()
+ project.version = '1.0.0'
+
+ when:
+ project.plugins.apply('org.apache.grails.gradle.grails-publish')
+
+ then:
+ project.tasks.names.toList() == ["assemble", "build", "check",
"clean", "closeAndReleaseSonatypeStagingRepository",
+ "closeAndReleaseStagingRepositories",
"closeSonatypeStagingRepository", "closeStagingRepositories",
+ "findSonatypeStagingRepository",
"initializeSonatypeStagingRepository", "publish", "publishToMavenLocal",
+ "releaseSonatypeStagingRepository",
"releaseStagingRepositories", "retrieveSonatypeStagingProfile"]
Review Comment:
```suggestion
project.tasks.names.toList() == [
'assemble',
'build',
'check',
'clean',
'closeAndReleaseSonatypeStagingRepository',
'closeAndReleaseStagingRepositories',
'closeSonatypeStagingRepository',
'closeStagingRepositories',
'findSonatypeStagingRepository',
'initializeSonatypeStagingRepository',
'publish',
'publishToMavenLocal',
'releaseSonatypeStagingRepository',
'releaseStagingRepositories',
'retrieveSonatypeStagingProfile',
]
```
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -512,13 +546,16 @@ Note: if project properties are used, the properties must
be defined prior to ap
}
SourceSetContainer sourceSets = findSourceSets(project)
- if (!sourceSets.main.hasProperty('groovy')) {
+
+ SourceSet main = sourceSets.named('main').get()
+ GroovySourceDirectorySet groovy =
main.getExtensions().findByType(GroovySourceDirectorySet)
Review Comment:
```suggestion
def main = sourceSets.named('main').get()
def groovy = main.extensions.findByType(GroovySourceDirectorySet)
```
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -250,192 +271,158 @@ Note: if project properties are used, the properties
must be defined prior to ap
validateProjectPublishable(project as Project)
- project.publishing {
+ project.extensions.configure(PublishingExtension) {
PublishingExtension pe ->
final GrailsPublishExtension gpe =
extensionContainer.findByType(GrailsPublishExtension)
final def mavenPublishUrl =
project.findProperty('mavenPublishUrl') ?: System.getenv('MAVEN_PUBLISH_URL')
if (useMavenPublish) {
System.setProperty('org.gradle.internal.publish.checksums.insecure', true as
String)
- repositories {
- maven {
+ pe.repositories { RepositoryHandler repoHandler ->
+ repoHandler.maven { MavenArtifactRepository repo ->
final String mavenPublishUsername =
project.findProperty('mavenPublishUsername') ?:
System.getenv('MAVEN_PUBLISH_USERNAME')
final String mavenPublishPassword =
project.findProperty('mavenPublishPassword') ?:
System.getenv('MAVEN_PUBLISH_PASSWORD')
if (mavenPublishUsername && mavenPublishPassword) {
- credentials {
- username = mavenPublishUsername
- password = mavenPublishPassword
+ repo.credentials { PasswordCredentials
credentials ->
+ credentials.username = mavenPublishUsername
+ credentials.password = mavenPublishPassword
}
}
- url = mavenPublishUrl
+ repo.url = mavenPublishUrl
+ repo.name = 'maven'
}
def testRepoPath = gpe.testRepositoryPath.getOrNull()
if (testRepoPath) {
- maven {
- name = 'TestCaseMavenRepo'
- url = testRepoPath
+ repoHandler.maven { MavenArtifactRepository repo ->
+ repo.name = 'TestCaseMavenRepo'
+ repo.url = testRepoPath
}
}
}
} else {
// This is a local publish. Add the test case repository
if it's defined on the extension.
def testRepoPath = gpe.testRepositoryPath.getOrNull()
if (testRepoPath) {
- repositories {
- maven {
- name = 'TestCaseMavenRepo'
- url = testRepoPath
+ pe.repositories { RepositoryHandler repoHandler ->
+ repoHandler.maven { MavenArtifactRepository repo ->
+ repo.name = 'TestCaseMavenRepo'
+ repo.url = testRepoPath
}
}
}
}
- publications {
- it.create(gpe.publicationName.get(), MavenPublication) {
- delegate.artifactId = gpe.artifactId.get()
- delegate.groupId = gpe.groupId.get()
+ pe.publications { PublicationContainer publications ->
+ publications.create(gpe.publicationName.get(),
MavenPublication) { MavenPublication publication ->
+ publication.artifactId = gpe.artifactId.get()
+ publication.groupId = gpe.groupId.get()
if (gpe.addComponents.get()) {
- doAddArtefact(project, delegate)
+ doAddArtefact(project, publication)
def extraArtefact =
getDefaultExtraArtifact(project)
if (extraArtefact) {
- artifact extraArtefact
+ publication.artifact(extraArtefact)
}
}
- pom.withXml {
- Node pomNode = asNode()
-
- if
(!project.extensions.findByType(JavaPlatformExtension)) {
- // Prevent multiple dependencyManagement nodes
- if (pomNode.dependencyManagement) {
-
pomNode.dependencyManagement[0].replaceNode {}
- }
- }
-
- if (gpe != null) {
- pomNode.children().last() + {
- delegate.name gpe.title.get()
- delegate.description gpe.desc.get()
- delegate.url gpe.websiteUrl.get()
-
- def license = gpe.license
- if (license != null) {
- def concreteLicense =
GrailsPublishExtension.License.LICENSES.get(license.name)
- if (concreteLicense != null) {
- delegate.licenses {
- delegate.license {
- delegate.name
concreteLicense.name
- delegate.url
concreteLicense.url
- delegate.distribution
concreteLicense.distribution
- }
- }
- } else if (license.name &&
license.url) {
- delegate.licenses {
- delegate.license {
- delegate.name license.name
- delegate.url license.url
- delegate.distribution
license.distribution
- }
- }
+ publication.pom { MavenPom pom ->
+ pom.name.set(gpe.title.get())
+ pom.description.set(gpe.desc.get())
+ pom.url.set(gpe.websiteUrl.get())
Review Comment:
```suggestion
pom.name.set(gpe.title)
pom.description.set(gpe.desc)
pom.url.set(gpe.websiteUrl)
```
Is a `Provider` acceptable as a parameter to `.set()` here?
##########
plugin/src/test/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePluginTest.groovy:
##########
@@ -45,6 +98,37 @@ class GrailsPublishGradlePluginTest extends Specification {
project.plugins.apply('org.apache.grails.gradle.grails-publish')
then:
- project.tasks.findByName('publish') != null
+ project.tasks.names.toList() == ['publish', 'publishToMavenLocal']
+ }
+
+ def 'evaluate: plugin registers release task for snapshot version'() {
+ given:
+ def project = ProjectBuilder.builder().build()
+ project.version = '1.0.0-SNAPSHOT'
+
+ and:
+ project.plugins.apply('org.apache.grails.gradle.grails-publish')
+ project.plugins.apply('java')
+
+ and:
+ GrailsPublishExtension gpe =
project.extensions.getByType(GrailsPublishExtension)
+ gpe.githubSlug.set('apache/grails-gradle-publish')
+ gpe.license {
+ name = 'Apache-2.0'
+ }
+ gpe.title.set('Grails Gradle Publish Plugin')
+ gpe.desc.set('A plugin to assist in publishing Grails artifacts')
+ gpe.developers.set(['jdaugherty': 'James Daugherty'])
+
+ when:
+ ((ProjectInternal) project).evaluate()
+
+ then:
+ project.tasks.names.toList() == ["artifactTransforms", "assemble",
"build", "buildDependents", "buildEnvironment", "buildNeeded", "check",
"classes", "clean", "compileJava",
+ "compileTestJava", "components",
"dependencies", "dependencyInsight", "dependentComponents",
"generateMetadataFileForMavenPublication",
+ "generatePomFileForMavenPublication",
"grailsPublishValidation", "help", "init", "install", "jar", "javaToolchains",
"javadoc", "javadocJar",
+ "model", "outgoingVariants",
"processResources", "processTestResources", "projects", "properties", "publish",
+
"publishAllPublicationsToMavenRepository",
"publishMavenPublicationToMavenLocal",
"publishMavenPublicationToMavenRepository",
+ "publishToMavenLocal",
"resolvableConfigurations", "sourcesJar", "tasks", "test", "testClasses",
"testSourcesJar", "updateDaemonJvm", "wrapper"]
Review Comment:
```suggestion
project.tasks.names.toList() == [
'artifactTransforms',
'assemble',
'build',
'buildDependents',
'buildEnvironment',
'buildNeeded',
'check',
'classes',
'clean',
'compileJava',
'compileTestJava',
'components',
'dependencies',
'dependencyInsight',
'dependentComponents',
'generateMetadataFileForMavenPublication',
'generatePomFileForMavenPublication',
'grailsPublishValidation',
'help',
'init',
'install',
'jar',
'javaToolchains',
'javadoc',
'javadocJar',
'model',
'outgoingVariants',
'processResources',
'processTestResources',
'projects',
'properties',
'publish',
'publishAllPublicationsToMavenRepository',
'publishMavenPublicationToMavenLocal',
'publishMavenPublicationToMavenRepository',
'publishToMavenLocal',
'resolvableConfigurations',
'sourcesJar',
'tasks',
'test',
'testClasses',
'testSourcesJar',
'updateDaemonJvm',
'wrapper',
]
```
##########
plugin/src/test/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePluginTest.groovy:
##########
@@ -19,24 +19,77 @@
package org.apache.grails.gradle.publish
+import org.gradle.api.GradleException
+import org.gradle.api.internal.project.ProjectInternal
import org.gradle.testfixtures.ProjectBuilder
import spock.lang.Specification
class GrailsPublishGradlePluginTest extends Specification {
- def 'plugin registers release task for release version'() {
+ def "requires java or java platform plugin"() {
Review Comment:
```suggestion
def 'requires java or java platform plugin'() {
```
##########
plugin/src/test/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePluginTest.groovy:
##########
@@ -19,24 +19,77 @@
package org.apache.grails.gradle.publish
+import org.gradle.api.GradleException
+import org.gradle.api.internal.project.ProjectInternal
import org.gradle.testfixtures.ProjectBuilder
import spock.lang.Specification
class GrailsPublishGradlePluginTest extends Specification {
- def 'plugin registers release task for release version'() {
+ def "requires java or java platform plugin"() {
given:
def project = ProjectBuilder.builder().build()
project.version = '1.0.0'
when:
project.plugins.apply('org.apache.grails.gradle.grails-publish')
+ ((ProjectInternal) project).evaluate()
then:
- project.tasks.findByName('retrieveSonatypeStagingProfile') != null
+ def ge = thrown(GradleException)
+ ge.cause.message == "Grails Publish Plugin requires the Java Platform
or Java Plugin to be applied to the project."
}
- def 'plugin registers release task for snapshot version'() {
+ def 'apply only: plugin registers release task for release version'() {
+ given:
+ def project = ProjectBuilder.builder().build()
+ project.version = '1.0.0'
+
+ when:
+ project.plugins.apply('org.apache.grails.gradle.grails-publish')
+
+ then:
+ project.tasks.names.toList() == ["assemble", "build", "check",
"clean", "closeAndReleaseSonatypeStagingRepository",
+ "closeAndReleaseStagingRepositories",
"closeSonatypeStagingRepository", "closeStagingRepositories",
+ "findSonatypeStagingRepository",
"initializeSonatypeStagingRepository", "publish", "publishToMavenLocal",
+ "releaseSonatypeStagingRepository",
"releaseStagingRepositories", "retrieveSonatypeStagingProfile"]
+ }
+
+ def "evaluate: plugin registers release task for release version"() {
Review Comment:
```suggestion
def 'evaluate: plugin registers release task for release version'() {
```
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -59,7 +80,7 @@ class GrailsPublishGradlePlugin implements Plugin<Project> {
public static String SNAPSHOT_PUBLISH_TYPE_PROPERTY = 'snapshotPublishType'
public static String RELEASE_PUBLISH_TYPE_PROPERTY = 'releasePublishType'
- String getErrorMessage(String missingSetting) {
+ static String getErrorMessage(String missingSetting) {
Review Comment:
```suggestion
static String createErrorMessage(String missingSetting) {
```
Try to reserve the get* method name idiom for getters?
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -512,13 +546,16 @@ Note: if project properties are used, the properties must
be defined prior to ap
}
SourceSetContainer sourceSets = findSourceSets(project)
- if (!sourceSets.main.hasProperty('groovy')) {
+
+ SourceSet main = sourceSets.named('main').get()
+ GroovySourceDirectorySet groovy =
main.getExtensions().findByType(GroovySourceDirectorySet)
+ if (!groovy) {
return null
}
- String pluginXml =
"${sourceSets.main.groovy.getClassesDirectory().get().getAsFile()}/META-INF/grails-plugin.xml".toString()
- new File(pluginXml).exists() ? [
- source : pluginXml,
+ File pluginXml =
groovy.getClassesDirectory().get().file('META-INF/grails-plugin.xml').asFile
Review Comment:
```suggestion
def pluginXml =
groovy.classesDirectory.get().file('META-INF/grails-plugin.xml').asFile
```
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -529,15 +566,11 @@ Note: if project properties are used, the properties must
be defined prior to ap
}
protected validateProjectPublishable(Project project) {
- boolean hasJavaPlugin =
project.extensions.findByType(JavaPluginExtension)
- boolean hasJavaPlatform =
project.extensions.findByType(JavaPlatformExtension)
+ boolean hasJavaPlugin =
project.extensions.findByType(JavaPluginExtension) as JavaPlatformExtension
+ boolean hasJavaPlatform =
project.extensions.findByType(JavaPlatformExtension) as JavaPlatformExtension
Review Comment:
```suggestion
def javaPlugin = project.extensions.findByType(JavaPluginExtension)
def javaPlatform =
project.extensions.findByType(JavaPlatformExtension)
```
and the corresponding variable renames below.
##########
plugin/src/test/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePluginTest.groovy:
##########
@@ -19,24 +19,77 @@
package org.apache.grails.gradle.publish
+import org.gradle.api.GradleException
+import org.gradle.api.internal.project.ProjectInternal
import org.gradle.testfixtures.ProjectBuilder
import spock.lang.Specification
class GrailsPublishGradlePluginTest extends Specification {
- def 'plugin registers release task for release version'() {
+ def "requires java or java platform plugin"() {
given:
def project = ProjectBuilder.builder().build()
project.version = '1.0.0'
when:
project.plugins.apply('org.apache.grails.gradle.grails-publish')
+ ((ProjectInternal) project).evaluate()
then:
- project.tasks.findByName('retrieveSonatypeStagingProfile') != null
+ def ge = thrown(GradleException)
+ ge.cause.message == "Grails Publish Plugin requires the Java Platform
or Java Plugin to be applied to the project."
}
- def 'plugin registers release task for snapshot version'() {
+ def 'apply only: plugin registers release task for release version'() {
+ given:
+ def project = ProjectBuilder.builder().build()
+ project.version = '1.0.0'
+
+ when:
+ project.plugins.apply('org.apache.grails.gradle.grails-publish')
+
+ then:
+ project.tasks.names.toList() == ["assemble", "build", "check",
"clean", "closeAndReleaseSonatypeStagingRepository",
+ "closeAndReleaseStagingRepositories",
"closeSonatypeStagingRepository", "closeStagingRepositories",
+ "findSonatypeStagingRepository",
"initializeSonatypeStagingRepository", "publish", "publishToMavenLocal",
+ "releaseSonatypeStagingRepository",
"releaseStagingRepositories", "retrieveSonatypeStagingProfile"]
+ }
+
+ def "evaluate: plugin registers release task for release version"() {
+ given:
+ def project = ProjectBuilder.builder().build()
+ project.version = '1.0.0'
+
+ and:
+ project.plugins.apply('org.apache.grails.gradle.grails-publish')
+ project.plugins.apply('java')
+
+ and:
+ GrailsPublishExtension gpe =
project.extensions.getByType(GrailsPublishExtension)
+ gpe.githubSlug.set('apache/grails-gradle-publish')
+ gpe.license {
+ name = 'Apache-2.0'
+ }
+ gpe.title.set('Grails Gradle Publish Plugin')
+ gpe.desc.set('A plugin to assist in publishing Grails artifacts')
+ gpe.developers.set(['jdaugherty': 'James Daugherty'])
+
+ when:
+ ((ProjectInternal) project).evaluate()
+
+ then:
+ project.tasks.names.toList() == ["artifactTransforms", "assemble",
"build", "buildDependents", "buildEnvironment", "buildNeeded", "check",
"classes", "clean",
+
"closeAndReleaseSonatypeStagingRepository",
"closeAndReleaseStagingRepositories", "closeSonatypeStagingRepository",
"closeStagingRepositories",
+ "compileJava", "compileTestJava",
"components", "dependencies", "dependencyInsight", "dependentComponents",
"findSonatypeStagingRepository",
+
"generateMetadataFileForMavenPublication",
"generatePomFileForMavenPublication", "grailsPublishValidation", "help", "init",
+
"initializeSonatypeStagingRepository", "install", "jar", "javaToolchains",
"javadoc", "javadocJar", "model", "outgoingVariants",
+ "processResources",
"processTestResources", "projects", "properties", "publish",
"publishAllPublicationsToSonatypeRepository",
+
"publishMavenPublicationToMavenLocal",
"publishMavenPublicationToSonatypeRepository", "publishToMavenLocal",
"publishToSonatype",
+ "releaseSonatypeStagingRepository",
"releaseStagingRepositories", "resolvableConfigurations",
"retrieveSonatypeStagingProfile",
+ "signMavenPublication", "sourcesJar",
"tasks", "test", "testClasses", "testSourcesJar", "updateDaemonJvm", "wrapper"]
Review Comment:
```suggestion
project.tasks.names.toList() == [
'artifactTransforms',
'assemble',
'build',
'buildDependents',
'buildEnvironment',
'buildNeeded',
'check',
'classes',
'clean',
'closeAndReleaseSonatypeStagingRepository',
'closeAndReleaseStagingRepositories',
'closeSonatypeStagingRepository',
'closeStagingRepositories',
'compileJava',
'compileTestJava',
'components',
'dependencies',
'dependencyInsight',
'dependentComponents',
'findSonatypeStagingRepository',
'generateMetadataFileForMavenPublication',
'generatePomFileForMavenPublication',
'grailsPublishValidation',
'help',
'init',
'initializeSonatypeStagingRepository',
'install',
'jar',
'javaToolchains',
'javadoc',
'javadocJar',
'model',
'outgoingVariants',
'processResources',
'processTestResources',
'projects',
'properties',
'publish',
'publishAllPublicationsToSonatypeRepository',
'publishMavenPublicationToMavenLocal',
'publishMavenPublicationToSonatypeRepository',
'publishToMavenLocal',
'publishToSonatype',
'releaseSonatypeStagingRepository',
'releaseStagingRepositories',
'resolvableConfigurations',
'retrieveSonatypeStagingProfile',
'signMavenPublication',
'sourcesJar',
'tasks',
'test',
'testClasses',
'testSourcesJar',
'updateDaemonJvm',
'wrapper',
]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
