jamesfredley commented on PR #15713: URL: https://github.com/apache/grails-core/pull/15713#issuecomment-4732882027
I re-verified my earlier SM2 parity list against the current PR head and addressed the outstanding CI item. Most items have been resolved by the commits pushed since my last review; status below. ## SM2 parity list - current status | Item | Status | Notes | | --- | --- | --- | | `g:applyLayout` attribute parity | ✅ Addressed | `RenderSitemeshTagLib` now handles `name`, `template`/`view`, `url`, `action`+`controller`, `params`, `model`, `contentType`, and `parse` (commit "Support the full applyLayout attribute surface under SiteMesh 3"). | | Default layout config + implicit `application` fallback | ✅ Addressed | `Sitemesh3GrailsPlugin` reads `grails.sitemesh.default.layout` then falls back to `grails.views.layout.default`; `Sitemesh3LayoutFinder.resolveDefaultDecorator()` falls back to the implicit `application` layout when none is configured (commit "Match SiteMesh 2's default layout config and application fallback"). | | `<title>` stripping robustness | ✅ Addressed | `Sitemesh3CapturedPage.indexOfTitleOpenTag()` now requires the char after `<title` to be `>` or whitespace, so `<titlebar>`/`<title-x>` are no longer mis-sliced (commit "Don't mis-slice head content around title-prefixed elements"). | | Test examples default path (Hibernate 7) | ✅ Addressed | The hibernate7 examples now use the `SITEMESH2_TESTING_ENABLED` toggle and default to SM3 when unset, consistent with the rest of the examples. | | SM2-only examples without SM3 coverage | ✅ Addressed | `scaffolding` and `jetty` now use the `SITEMESH2_TESTING_ENABLED` toggle (default SM3) rather than being hardcoded to `grails-layout`. | | `grails-gsp-spring-boot` publication | ✅ Addressed | Now listed in `gradle/publish-root-config.gradle` alongside `grails-sitemesh3`. | | Upstream auto-config suppression guard | ⚠️ Mechanism replaced | The disabled `NoopSitemeshFilter` workaround was dropped. Suppression is now done in `Sitemesh3AutoConfiguration` via `@AutoConfigureBefore(SiteMeshViewResolverAutoConfiguration)` + `@ConditionalOnMissingBean(SiteMeshViewResolverBeanPostProcessor)`, covered by `GrailsSiteMeshViewResolverBeanPostProcessorSpec`. There is still no dedicated test asserting the upstream *filter* auto-config stays suppressed, but the original fragile bean is gone. | | `grails.views.layout.enable.nongsp` | ❌ Not addressed | `Sitemesh3LayoutFinder` still resolves only through the GSP locator; the SM2 `enableNonGspViews` / non-GSP (JSP) layout flag has no SM3 equivalent. Low priority - can be a follow-up if non-GSP layouts need to be supported under SM3. | | CI coverage for SM2 lane | ✅ Addressed here | See below. | ## SM2 CI lane Per @jdaugherty's suggestion (a separate workflow to run the SiteMesh 2 work, since legacy apps may still use it and it isn't exercised by default), I added `.github/workflows/sitemesh2.yml`. It sets `SITEMESH2_TESTING_ENABLED=true` - which flips the starter, the uber unit suite and the functional test examples back onto `:grails-layout` - and runs the core (`-PonlyCoreTests`) and functional (`-PonlyFunctionalTests`) lanes against it. `--rerun-tasks` ensures the tests execute against the grails-layout classpath rather than reusing SM3-cached outputs. Triggers: `workflow_dispatch`, weekly `schedule`, and `push`/`pull_request` scoped to the GSP / test-suite / test-example / dependency paths where SM2-vs-SM3 behavior lives. One caveat worth noting: GitHub only runs `schedule` and `workflow_dispatch` from the repository default branch, so the weekly/manual lane will not fire from an 8.x branch until 8.x is the default (PR and push coverage is unaffected). Remaining open items are `grails.views.layout.enable.nongsp` and an explicit upstream-filter-suppression guard test, both of which can be deferred as follow-ups. -- 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]
