jamesfredley commented on PR #15557: URL: https://github.com/apache/grails-core/pull/15557#issuecomment-4606313303
### Review feedback addressed - 8 commits pushed (`fb725b178b..caad5429a0`) Worked through every open review thread. Each commit is scoped to one concern; resolved threads link their commit for traceability. **Reverted as out of scope (no Groovy 5 change needed):** - `369d674597` - reverted the manual controller-action parameter rewrites back to `def index(Integer max)` form. Parameterized actions work via `ControllerActionTransformer`'s `OptimizingStatementWriter.ClassNodeSkip` tag (`fb725b178b`), so the `params.int(...)` rewrites were redundant. - `df4750c506` - removed the per-app `allowedBomOverrides`; pinned `org.ow2.asm:asm`/`asm-util` at `9.10.1` in the micronaut BOM so the resolved version matches the BOM (the validator's match-or-exclude contract), and dropped the redundant micronaut `groovy.version` override. **Fixes:** - `013becf53e` - `hibernate.configClass` now binds under Groovy 5. `ConfigurationBuilder` resolves `Class`-typed settings via the context class loader (handles both `Class` literals and class-name Strings), independent of the load-order-sensitive String->Class converter that resolved against the framework class loader. BootStrap assert re-enabled; covered by three new specs. - `3fbfc1aa4e` - restored Groovy truth on the `template` argument in `TemplateRendererImpl`. - `9dd79859c9` - re-enabled `JsonViewTemplateResolverSpec`: the disabled spec stubbed the final `GrailsWebRequest.getRequest()` the resolver never calls, so the stub was dropped - no mocking library needed. - `c884184677` - explicit global `GroovySpy` cleanup in `DataBindingTests`. **Docs / comments:** - `caad5429a0` - new "Apache Groovy 5 Behavior Changes" section in the upgrade guide (static-field assignment in a static closure now needs the class-name qualifier; `ConfigObject` subscript auto-vivifies missing keys). - `d67db7e8d6` - clarified/trimmed the Groovy 5 workaround comments; the trait static-method reflection workaround now cites [GROOVY-11985](https://issues.apache.org/jira/browse/GROOVY-11985). **Still open for your input** (explained on each thread rather than worked around blindly): - GSP `@CompileStatic` undeclared-variable detection specs - regressed in Groovy 5 STC (GROOVY-6362 / GROOVY-11817 / GROOVY-12041). - The `VariableScopeVisitor` null-exceptions chokepoint proxy (upstream Groovy NPE), `@Sortable`+`@Entity` interaction, the geb STC accessor, `@Slf4j`-applied-in-transform, and the `DefaultViewRenderer` generics tightening - a few flagged for @paulk-asert. - Two items that genuinely cannot be reverted, with rationale inline: the `$tt__` direct-invocation assertions (Spock 2.4 reads the per-iteration context, which is unavailable off the runner) and the `TestTrait<F extends Serializable>` bound (the bound makes Groovy 5 generate an abstract trait setter on the implementer). Goal throughout was no change where possible, and where a change/workaround was unavoidable, the most concrete one - aligned with the guidance on these threads. -- 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]
