codeconsole commented on PR #15939:
URL: https://github.com/apache/grails-core/pull/15939#issuecomment-4926789910
Thanks — you're right, the description had drifted ahead of the diff. I took
your option 2 and narrowed both the description and the scope: the hook lives
on `grails.plugins.Plugin` and is dispatched by `DefaultGrailsPlugin` for
plugin descriptors only. I've updated the PR description to match.
For the record on why not option 1 (wire it into
`GrailsApplicationLifeCycle`): an earlier revision of this PR did exactly that
— a `default` method on the interface, an override on
`GrailsApplicationLifeCycleAdapter`, and dispatch from
`GrailsApplicationPostProcessor`. It broke CI across every module that builds
an app context in a unit test. The testing support map-coerces the interface:
```groovy
// GrailsApplicationBuilder
super([doWithSpring: { -> doWithSpringClosure }] as
GrailsApplicationLifeCycle, ...)
```
and Groovy's map-to-interface proxies don't honour Java `default` methods,
so `lifeCycle.doWithSpring(bb)` threw `MissingMethodException` for the whole
`GrailsUnitTest` path. So the interface-level version isn't a small follow-up —
it needs the testing support reworked off map coercion first. Scoping to
`Plugin` keeps this PR focused and safe; extending it to the application class
can be a separate change.
Separately, per @jdaugherty's review, defining both hook forms is now a hard
error (`PluginException`) rather than running both.
--
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]
