borinquenkid commented on PR #15568:
URL: https://github.com/apache/grails-core/pull/15568#issuecomment-4530728883
> I believe I have explanations or resolutions to all of my critical issues.
These 3 remain:
>
> 1. grails-gsp/.../GroovyPagesServlet.java
> A change to the thread context class loader flagged by PMD. I think
it's probably fine but this has caused issues historically. I'll leave the
conversation unresolved in case @davydotcom wants to respond.
> 2. grails-datastore-core/.../ConnectionSource.java
> The default datastore name was changed. I am OK with it but I'd like to
understand the rationale — why was it necessary to change the default?
> 3. I left a TODO in the mongo doc build since we can't include hibernate5
& hibernate7. Hopefully @jamesfredley has some feedback here.
1) Leaving this unresolved pending response from @davydotcom. PMD flags the
thread context class loader change but behavior is unchanged from H5; flagged
for committer review before merge.
2) Hibernate 7's HibernateConnectionSourceFactory registers its datastores
using the key "default" (lowercase). The old constant ConnectionSource.DEFAULT
= "DEFAULT" (uppercase) caused silent lookup
misses because HashMap key comparison is case-sensitive — the constant and
the registered key never matched. Changing the constant to "default" aligns it
with what H7 actually registers. Backward
compatibility is preserved two ways: OLD_DEFAULT = "DEFAULT" is kept
@Deprecated for any code referencing the old string literal, and
GormRegistry.normalizeQualifier() coerces any incoming "DEFAULT" to
"default" transparently via OLD_DEFAULT.equalsIgnoreCase().
3) 3. MongoDB doc TODO
This is a structural problem: the Grails doc build is a single pass and
can only pull from one GORM data module at a time (H5 or H7), but MongoDB is
independent of both. There are three realistic
options:
Option A — separate doc builds (recommended long-term)
Build the guide twice — once with H5 and once with H7 — publishing to
versioned paths (/docs/h5/, /docs/h7/). MongoDB docs live in a shared adoc
include that both pick up. This is the cleanest but
requires CI and site infra changes.
Option B — conditional AsciiDoc includes (quick fix)
Use AsciiDoc attributes/ifdef in the doc source to conditionally include
H5 vs H7 content, driven by a build flag. MongoDB content that applies to both
goes in a shared include file. This is what most
projects do when they can't split the build yet.
Option C — link out to standalone GORM docs (unblock now)
Replace the TODO with a link to the standalone GORM for MongoDB docs
(published independently). Removes the coupling entirely. Weakest option but
unblocks the PR immediately.
@jamesfredley Is Option A (separate builds) is on the roadmap or Option B
(build flag) is acceptable for M2.
--
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]
