borinquenkid commented on PR #16102:
URL: https://github.com/apache/grails-core/pull/16102#issuecomment-5347277204
> Thanks @borinquenkid — I went through all five. Here is the rationale
behind all 5. Please LMK if you think they are still valid based on my response.
>
> **1. Cache duration 5s → 1s — I'd keep 5s.** It isn't arbitrary: it's the
historical Grails default. `I18nGrailsPlugin` set `field('cacheSeconds',
int).value(Settings.I18N_CACHE_SECONDS, '5')`, and this PR deliberately
preserves that so an upgrading application sees the same reload timing it had.
It's also what the deprecated `grails.i18n.cache.seconds` alias documents.
Anyone who prefers tighter feedback sets `spring.messages.cache-duration: 1s`,
which now works where it silently did nothing before.
Works for me
>
> **2. Multiple application descriptors → warn and merge — I'd keep the
error.** Merging doesn't remove the ambiguity, it hides it: two applications
both shipping `messages` would have their precedence decided by classpath
enumeration order, which is exactly the nondeterminism this design exists to
remove (and what @sbglasius flagged for plugin names earlier in this PR). The
scenario also isn't demonstrated — the full suite, 16k+ tests including every
multi-module test-example, passes with the check in place. The error already
names both descriptors and the likely cause. If a real multi-application
classpath turns up, I'd rather fix it with evidence in hand.
Work for me
>
> **3. Soften the BootstrapContext requirement — I don't think this case is
reachable, and the fallback would be worse.**
`GrailsBootstrapRegistryInitializer` is registered in `grails-core`'s
`META-INF/spring.factories` as a `BootstrapRegistryInitializer`, so
`SpringApplication` loads it automatically — `@SpringBootTest` included.
`ApplicationContextRunner` doesn't invoke `EnvironmentPostProcessor`s at all,
so the branch never runs there either. More importantly, the proposed fallback
of sorting alphabetically would silently produce _wrong_ precedence:
correctness here depends on reverse topological order, which is what reproduces
the behaviour of the message source this replaces. A warning plus wrong
messages is worse than a clear failure.
Works for me
>
> **4. Legacy plugin fallback — out of scope, deliberately.** A Grails 7
plugin JAR already doesn't work in Grails 8 for reasons unrelated to i18n; the
upgrade guide states it directly in several places — "the plugin must be
rebuilt against the typed metadata API", "the plugin must be rebuilt for Grails
8", "the plugin's trait must be recompiled with Groovy 5", and plugin authors
"must compile against Spring Framework 7". Adding a runtime classpath scan to
preserve message bundles for artifacts that can't load anyway would reintroduce
the exact scan this PR removes — the one thing that makes the i18n path
AOT-hostile — for no working use case. Plugins are recompiled for 8 regardless;
a namespaced bundle works unchanged under both 7 and 8 (documented in the
upgrade section), so the migration is a one-time rename.
Works for me.
>
> **5. Native image end-to-end verification — not currently possible.**
There is no native build path for a Grails application on `8.0.x` yet: no
`org.graalvm.buildtools` plugin, no `SpringBootAotPlugin`/`processAot` wiring,
and no native target in `end-to-end/`. That's what #16094 is for, and it's
still open. This PR doesn't claim native support works end to end — the
Limitations section says so explicitly, and calls out that the resource-pattern
choice follows Spring Boot's own precedent rather than being proven by a native
build. Once #16094 lands, an end-to-end native test is the right follow-up, and
this PR is what makes the i18n side ready for it.
Open an Issue in Grails for that.
--
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]
