jamesfredley commented on PR #15365:
URL: https://github.com/apache/grails-core/pull/15365#issuecomment-4320039393
## Review feedback addressed (commits ccafe2bd86 .. fe527d1419)
Pushed 8 atomic commits resolving every line comment from the latest review.
Net effect vs prior PR head: **+153 / -241 across 20 files**. All commits are
independently revertable.
### Commit ledger
| Commit | Concern |
|---|---|
| `0196bd2dd0` (matrei, via merge) | #15597 `build: bootstrap Gradle plugin
repositories` - shared `gradle/plugin-repositories.gradle` script (preserves
authorship) |
| `fe527d1419` | Merge of #15597 + drop the inline
`pluginManagement.repositories` blocks I had added across 6 settings files.
Also adopts matrei's refactor for `build-logic/settings.gradle` (deletes its
33-line inline block in favor of the shared script). All 7 settings files now
route through the same `configurePluginRepositories(delegate)` |
| `5d21fc3a39` | Drop 4 redundant explanatory comments flagged by
@jdaugherty (`gradle.properties`, `grails-bom/build.gradle`,
`GrailsGradlePlugin.groovy`, `GrailsProfileGradlePlugin.groovy`) |
| `dae431d4a1` | Drop forge `jakartaInjectVersion` override -
micronaut-platform 4.10.10 BOM manages it transitively (verified resolves to
2.0.1) |
| `014355310b` | Drop forge `groovyVersion` override and the `eachDependency
{ useVersion(groovyVersion) }` resolutionStrategy block - BOM-managed (resolves
to 4.0.28, the version Micronaut 4.10.10 was built against). spock-core
override preserved |
| `cc2f85fe07` | `gradle-groovy.version` derived from `GroovySystem.version`
instead of hard-coded `'4.0.31'`. Published BOM now contains
`<gradle-groovy.version>4.0.29</gradle-groovy.version>` (Gradle 9.4.1's bundled
Groovy) |
| `d9a218175a` | Make issue-11767 `mergeMicronautConfigMetadata` properly
cacheable - writes to separate `build/merged-spring-configuration-metadata/`
dir, declares lazy inputs/outputs, jar uses `eachFile` filter. Second
invocation now reports `UP-TO-DATE` |
| `ccafe2bd86` | Roll selenium 4.41.0 → 4.37.0 (Spring Boot 4.0.5 BOM
aligned) and drop the OpenTelemetry 1.59.0 override + 24-module constraint
list. The override existed only to bridge the selenium-4.41 ↔ spring-boot-4.0.5
opentelemetry conflict; aligning with Spring Boot's choice eliminates it. Net
diff in `dependencies.gradle`: -40 lines |
### Verification matrix
| Check | Result |
|---|---|
| `./gradlew help` (root + grails-gradle + grails-forge composites, JDK 21,
Gradle 9.4.1) | PASS |
| `./gradlew validateDependencyVersions` (whole project, 157 tasks) | PASS |
| `./gradlew :grails-bom:generatePomFileForMavenPublication` | PASS |
| `./gradlew :grails-gradle-bom:generatePomFileForMavenPublication` | PASS
(publishes `gradle-groovy.version=4.0.29`) |
| `./gradlew :grails-forge-{api,cli,core,test-core}:compileTestGroovy` |
PASS (groovy 4.0.28 from BOM) |
| `./gradlew :grails-forge-api:dependencies --configuration
compileClasspath` | jakarta-inject -> 2.0.1 via BOM |
| `./gradlew :grails-test-examples-plugins-issue-11767:clean :...:jar` |
PASS - jar contains 1 merged `META-INF/spring-configuration-metadata.json` with
both Java + Groovy beans |
| Second invocation of `:grails-test-examples-plugins-issue-11767:jar` | All
UP-TO-DATE (caching works) |
| `./gradlew :grails-test-examples-issue-11767:compileGroovy` (consumer) |
PASS |
| `./gradlew :grails-forge-analytics-postgres:assemble :...:dockerfile
:...:dockerfileNative` | PASS (no shadow plugin needed - CI uses
`dockerBuildNative`) |
| `./gradlew
:grails-test-examples-{geb,geb-context-path,geb-gebconfig}:compileTestGroovy` |
PASS (Selenium 4.37) |
### Resolves all open review threads
- 6× "use #15597 instead" on settings files → `fe527d1419`
- `dependencies.gradle:32` "GroovySystem version instead of hard coding" →
`cc2f85fe07`
- `dependencies.gradle:86` opentelemetry rationale → `ccafe2bd86` (deleted,
no longer needed)
- `dependencies.gradle:95` selenium pinning vs geb issues → `ccafe2bd86`
(rolled back to Spring Boot's 4.37)
- `gradle.properties:53` "Remove the comment" → `5d21fc3a39`
- `grails-bom/build.gradle:110` "this is bridge, remove it" → `5d21fc3a39`
- `grails-forge-analytics-postgres/build.gradle:21` shadow plugin removal →
verified safe (dockerBuildNative is the deploy path)
- `grails-forge-api/build.gradle:52` jakarta-inject mismatch concern →
`dae431d4a1`
- `test-core/build.gradle:62` followup on groovyVersion → `014355310b`
- `GrailsGradlePlugin.groovy:981` "remove the comment" → `5d21fc3a39`
- `GrailsProfileGradlePlugin.groovy:90` "remove the comment" → `5d21fc3a39`
- `issue-11767/build.gradle:77` cacheable merge → `d9a218175a`
Ready for re-review @jdaugherty @matrei.
Assisted-by: claude-code:claude-opus-4-7
--
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]
