codeconsole commented on PR #16399:
URL: https://github.com/apache/grails-core/pull/16399#issuecomment-5835584674
@matrei, thanks for the thorough review. These are addressed in 58b99742f1:
1. **Unconditional framework beans.** I kept the behaviour and narrowed the
docs, since an application's `@Bean` relates to an unconditional configuration
bean the same way. The guide now says a `beans` block replaces the harness's
stand-ins (such as `messageSource`) and framework beans declared
`@ConditionalOnMissingBean`. It also says it does not replace a framework
configuration's unconditional bean (such as `codecLookup`) or an included
plugin's bean, and points to `beanRegistrar()` for those.
`ReplaceFrameworkBeanSpec`, adapted from yours, pins all four hooks: the
`beans` block and the nested `@Configuration` get `DefaultCodecLookup`, while
`doWithSpring()` and `beanRegistrar()` replace it.
2. **Mixing hooks.** `MixedBeanHooksSpec`, adapted from yours, pins four
things: every hook contributes its beans; a shared name resolves
`beanRegistrar()` > `doWithSpring()` > `beans` block > nested `@Configuration`;
the generated class comes after the hand-written nested classes; and a
condition in the block does not see a `beanRegistrar()` bean, so both widgets
are registered. A new "Using more than one" section in the guide gives the
precedence, and says that a condition on one of the test's beans sees only the
test's configuration classes read before it. `BeansConfiguration` is now placed
after a test's hand-written nested classes explicitly (a stable sort per class
level), not by `getDeclaredClasses()` order.
3. **`@Shared` report.** It now fires only when the moved initializer is a
closure that declares beans (a `bean`/`field`/`method`/`group` call). A
`@Shared def beans = [someKey: 'someValue']` compiles as before. There's a spec
in `GlobalGrailsClassInjectorTransformationSpec`.
4. **Reclaim before claim.** The convention now reads the Spock-moved
closure where it is, via a new `movedInitializer`, and moves it back only once
the property is claimed. An unrelated `def beans = { … }` keeps its initializer
in `$spock_initializeFields`. The spec fails if the reclaim is moved back ahead
of the claim decision.
5. **Nested type names.** Added a note to the 8.0 upgrade guide:
`bean(Outer.Helper)` is now `helper`, not `outer$Helper`, so name it explicitly
if anything looks it up by the old name.
6. **Nits.** `@AutoConfiguration` on `IncludedBeansGrailsPlugin` is
required: a `Plugin` using `@GrailsBeans` must carry it, and the transform
moves it onto the generated sibling. A comment now says so.
`GrailsApplicationBuilder.configurationClasses` is now a `Set<Class<?>>`, like
the hook.
Locally I ran the specs touched: the testing-support specs,
`GlobalGrailsClassInjectorTransformationSpec` (56) and
`GrailsBeansASTTransformationSpec` (402), all passing, and `codeStyle` is clean
in all three modules. CI covers the rest.
--
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]
