-
-
Notifications
You must be signed in to change notification settings - Fork 962
feature - #14017 - add back Micronaut support to Grails 7 with optional plugin #14953
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 7.0.x
Are you sure you want to change the base?
Conversation
dc7a7d4
to
0952435
Compare
@amacleay-cohere @ysb33r @rlconst latest update on Micronaut support for Grails 7 |
3638832
to
b5c8513
Compare
9bfe90d
to
594b6eb
Compare
5f9bf1a
to
3fcdde0
Compare
3891886
to
37dba4e
Compare
@graemerocher or @sdelamo if either of you have any feedback on this change, it would be helpful. I am not as familiar with the micronaut project dependencies - for example, should the micronaut cache library still be included? I also took the approach that Grails 6 took: the gradle plugin has some logic to make it just "work" for the end user. The tests are limited in grails-core, but they are passing. My guess is we'll merge this and then make any incremental changes if there's specific feedback. |
065a8e0
to
09e06a3
Compare
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy
Outdated
Show resolved
Hide resolved
97ff0ec
to
6032ef1
Compare
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy
Show resolved
Hide resolved
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy
Show resolved
Hide resolved
grails-test-examples/micronaut/grails-app/init/micronaut/BootStrap.groovy
Outdated
Show resolved
Hide resolved
...est-examples/micronaut/src/integration-test/groovy/micronaut/BeanInjectionServiceSpec.groovy
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some questions and suggestions.
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy
Outdated
Show resolved
Hide resolved
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy
Outdated
Show resolved
Hide resolved
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy
Outdated
Show resolved
Hide resolved
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy
Outdated
Show resolved
Hide resolved
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy
Outdated
Show resolved
Hide resolved
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy
Outdated
Show resolved
Hide resolved
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy
Outdated
Show resolved
Hide resolved
f113686
to
9b4f0d3
Compare
…on like before
…e all view resolvers are lazy init
…re already context aware
f690c53
to
f64899b
Compare
The scaffolding failure was caused by the setters() having I'm still concerned about the lack of test coverage, other than the functional test apps, but I think users can try this out in the RC2 and we can go from there. @jamesfredley @matrei the bean wiring will likely have further issues - anywhere we use a setter with
A lot of these seem related to a change to support multiple message sources (438d2e0) Do we fix these to say it's supported? Prior versions of grails had a limited number of micronaut beans, so the grails beans weren't a micronaut bean by default. I'm skeptical people were accessing these beans from micronaut, so we may be able to make this change gradually. |
It seems moving to constructor injection created another issue? |
Odd, this didn't fail locally when I ran through all of the tests. Will take a look. |
…equire it as an exported dependency
@matrei This should be fixed now (https://ge.grails.org/s/6nvk6sdfjpzre) |
The forge failures are b/c of the asset pipeline depending on the grails-dependencies-web and we have moved the i18n plugin. I'd rather merge this + publish a new asset pipeline + update the version of the asset pipeline than try to add an exclude for that library. @jamesfredley @matrei thoughts? |
@jdaugherty Since you need this merged and published to use the new i18n coordinates to then update and release asset-pipeline, I think we can ignore the failures. |
I'll publish an asset pipeline release once we get sign off on this then. @matrei / @jamesfredley let me know when you think this is ready. |
Fixes #14017
grails-micronaut
as a dependency andmicronautPlatformVersion
as a gradle property