Copilot commented on code in PR #25:
URL:
https://github.com/apache/grails-gradle-publish/pull/25#discussion_r3126496908
##########
plugin/src/test/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePluginTest.groovy:
##########
@@ -197,10 +194,8 @@ class GrailsPublishGradlePluginTest extends Specification {
'clean',
'compileJava',
'compileTestJava',
- 'components',
'dependencies',
'dependencyInsight',
Review Comment:
Same brittleness concern as the other scenario: this test relies on exact
equality of the complete Gradle task list, so changes in Gradle core tasks will
keep breaking the test across Gradle releases. Prefer asserting the
presence/absence of the plugin's tasks (or comparing against a filtered subset)
so the test verifies plugin behavior rather than Gradle internals.
##########
plugin/src/test/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePluginTest.groovy:
##########
@@ -107,10 +107,8 @@ class GrailsPublishGradlePluginTest extends Specification {
'closeStagingRepositories',
'compileJava',
'compileTestJava',
- 'components',
'dependencies',
'dependencyInsight',
Review Comment:
This assertion is based on exact equality of the entire
`project.tasks.names` list, which is tightly coupled to Gradle's built-in task
set. The Gradle 9 upgrade already required updating this list; future Gradle
upgrades are likely to cause more churn. Consider asserting only the
plugin-specific tasks (and any intentionally introduced tasks) rather than the
full task list to keep the test focused and less brittle.
##########
plugin/build.gradle:
##########
@@ -27,10 +27,10 @@ group = 'org.apache.grails.gradle'
dependencies {
// compile grails-gradle-plugin with the Groovy version provided by Gradle
- // to ensure build compatibility with Gradle, currently Groovy 3.0.x
+ // to ensure build compatibility with Gradle, currently Groovy 4.0.x
// see: https://docs.gradle.org/current/userguide/compatibility.html#groovy
- api platform("org.codehaus.groovy:groovy-bom:${GroovySystem.version}")
- compileOnly 'org.codehaus.groovy:groovy'
+ api platform("org.apache.groovy:groovy-bom:${GroovySystem.version}")
Review Comment:
The Groovy BOM is added on the `api` configuration, which will be published
as part of this plugin's declared API and can leak Groovy version constraints
to consumers. Since this BOM is only needed to align versions for compiling the
plugin itself, prefer attaching it to a non-exported configuration (e.g.,
`compileOnly` / `implementation`) or using dependency constraints scoped to the
compile classpath so consumers aren't affected.
```suggestion
compileOnly
platform("org.apache.groovy:groovy-bom:${GroovySystem.version}")
```
--
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]
