You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "codeconsole (via GitHub)" <gi...@apache.org> on 2026/06/18 00:03:11 UTC
[PR] Convert RenderSitemeshTagLib tags to method handlers [grails-core]
codeconsole opened a new pull request, #15746:
URL: https://github.com/apache/grails-core/pull/15746
## Summary
Follow-up to #15713 addressing review feedback on `RenderSitemeshTagLib`:
> To make this faster, you should use the method version instead of closure.
Converts the seven closure-based tags in `RenderSitemeshTagLib` — `applyLayout`, `pageProperty`, `ifPageProperty`, `layoutTitle`, `layoutHead`, `layoutBody`, and `content` — to method handlers, so GSP dispatch uses the faster reflective method-invocation path introduced in #15465 instead of cloning a closure on every tag invocation. This also clears the closure-based-tag deprecation warning the #15465 compiler check emits for this class.
## Behavior preservation
Only the seven tag declarations change; the tag bodies are untouched. GSP dispatch (`TagOutput.captureTagOutput`) already passes `TagOutput.EMPTY_BODY_CLOSURE` (never `null`) as the body to two-argument tags, so the existing `body` / `body instanceof Closure` checks resolve identically whether each tag is a closure field or a method. The closure and method paths share the same output-stack setup, so `out` resolution is unchanged.
## Scope
`Sitemesh3LayoutTagLib` is intentionally left as closures, matching its SiteMesh 2 twin `GrailsLayoutTagLib`: those `grailsLayout`-namespace, `@CompileStatic` capture tags are driven by `GrailsLayoutPreprocessor` and were deliberately kept as closures in #15465.
## Tests
- Adds `RenderSitemeshTagLibSpec`, which verifies via the framework `TagMethodInvoker` API that all seven tags are discovered as invokable method handlers and no longer exist as closure fields.
- Existing end-to-end coverage in `grails-test-examples/gsp-sitemesh3` (`EndToEndSpec`) continues to exercise the runtime behavior of these tags.
## Validation
- `./gradlew :grails-sitemesh3:test` — green
- `./gradlew :grails-sitemesh3:checkstyleMain :grails-sitemesh3:codenarcMain` — green
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15746:
URL: https://github.com/apache/grails-core/pull/15746#issuecomment-4763470062
## ✅ All tests passed ✅
🏷️ Commit: 7a17e364918a425cb6ef6808499d6c4f1cbde9cc
▶️ Tests: 49255 executed
⚪️ Checks: 46/46 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15746:
URL: https://github.com/apache/grails-core/pull/15746#issuecomment-4764549579
## ✅ All tests passed ✅
🏷️ Commit: 36d9d3920cdc23f2891a33e91ee71fc748260ff9
▶️ Tests: 49343 executed
⚪️ Checks: 46/46 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1 [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on code in PR #15746:
URL: https://github.com/apache/grails-core/pull/15746#discussion_r3437008718
##########
grails-forge/grails-forge-core/src/main/java/org/grails/forge/build/gradle/GradleRepository.java:
##########
@@ -72,7 +72,7 @@ static Set<GradleRepository> getDefaultRepositories(String grailsVersion) {
null,
List.of(
new VersionRegexRepoFilter(
- "org[.]sitemesh.*", ".*", ".*-SNAPSHOT"
+ "org[.]sitemesh.*", ".*", ".*"
Review Comment:
@jdaugherty per @matrei it's redundant and not needed. https://github.com/apache/grails-core/pull/15710
##########
grails-shell-cli/src/main/groovy/org/grails/cli/profile/commands/CreateAppCommand.groovy:
##########
@@ -519,7 +519,7 @@ class CreateAppCommand extends ArgumentCompletingCommand implements ProfileRepos
configuredRepositories.add(repository)
GrailsGradleRepository sitemeshSnapshots = new GrailsGradleRepository(url: 'https://central.sonatype.com/repository/maven-snapshots', snapshotsOnly: true)
- sitemeshSnapshots.includeOnly('org[.]sitemesh.*', '.*', '.*-SNAPSHOT')
+ sitemeshSnapshots.includeOnly('org[.]sitemesh.*', '.*', '.*')
Review Comment:
@jdaugherty per @matrei it's redundant and not needed. https://github.com/apache/grails-core/pull/15710
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15746:
URL: https://github.com/apache/grails-core/pull/15746#issuecomment-4763974454
## ✅ All tests passed ✅
🏷️ Commit: a23ffee12e9fcc9c9d1e4954587986ff6dab14a4
▶️ Tests: 49336 executed
⚪️ Checks: 46/46 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15746:
URL: https://github.com/apache/grails-core/pull/15746#issuecomment-4737149398
## ✅ All tests passed ✅
🏷️ Commit: 87d2130b8ec35efc52f745614e4bc0388b3e7a77
▶️ Tests: 43149 executed
⚪️ Checks: 46/46 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1 [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole merged PR #15746:
URL: https://github.com/apache/grails-core/pull/15746
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15746:
URL: https://github.com/apache/grails-core/pull/15746#issuecomment-4745494323
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI - Groovy Joint Validation Build / build\_grails](https://github.com/apache/grails-core/actions/runs/27780397670/job/82204004301?pr=15746) > :grails-test-examples-scaffolding:integrationTest
| Test | Runs | Flakiness |
|---|---|---|
| UserControllerSpec > User list | ❌ |
🏷️ Commit: 614cc29d808943f930cbacca6faa3d222f05b5a0
▶️ Tests: 31619 executed
🟡 Checks: 45/46 completed
### Test Failures
<details>
<summary><strong>UserControllerSpec > User list</strong> (:grails-test-examples-scaffolding:integrationTest in <a href="https://github.com/apache/grails-core/actions/runs/27780397670/job/82204004301?pr=15746">CI - Groovy Joint Validation Build / build_grails</a>)</summary>
```
geb.waiting.WaitTimeoutException: condition did not pass in 30 seconds (failed with exception)
at geb.waiting.Wait.waitFor(Wait.groovy:128)
at geb.waiting.DefaultWaitingSupport.doWaitFor(DefaultWaitingSupport.groovy:55)
at geb.waiting.DefaultWaitingSupport.waitFor(DefaultWaitingSupport.groovy:45)
at geb.Page.waitFor(Page.groovy:120)
at com.example.pages.LoginPage.login(LoginPage.groovy:39)
at com.example.UserControllerSpec.setup(UserControllerSpec.groovy:33)
Caused by: Assertion failed:
title != pageTitle
| | |
| | 'Please sign in'
| false
'Please sign in'
at com.example.pages.LoginPage.login_closure1(LoginPage.groovy:39)
at com.example.pages.LoginPage.login_closure1(LoginPage.groovy)
at geb.waiting.Wait.waitFor(Wait.groovy:117)
... 5 more
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 UserControllerSpec > User list <!-- uniqueId=[engine:spock]/[spec:com.example.UserControllerSpec]/[feature:$spock_feature_1_0] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Convert SiteMesh 3 taglibs to method handlers and use SiteMesh 3.3.0-M1 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15746:
URL: https://github.com/apache/grails-core/pull/15746#discussion_r3433022373
##########
grails-shell-cli/src/main/groovy/org/grails/cli/profile/commands/CreateAppCommand.groovy:
##########
@@ -519,7 +519,7 @@ class CreateAppCommand extends ArgumentCompletingCommand implements ProfileRepos
configuredRepositories.add(repository)
GrailsGradleRepository sitemeshSnapshots = new GrailsGradleRepository(url: 'https://central.sonatype.com/repository/maven-snapshots', snapshotsOnly: true)
- sitemeshSnapshots.includeOnly('org[.]sitemesh.*', '.*', '.*-SNAPSHOT')
+ sitemeshSnapshots.includeOnly('org[.]sitemesh.*', '.*', '.*')
Review Comment:
Same
comment, I don't think this needs to change
##########
grails-forge/grails-forge-core/src/main/java/org/grails/forge/build/gradle/GradleRepository.java:
##########
@@ -72,7 +72,7 @@ static Set<GradleRepository> getDefaultRepositories(String grailsVersion) {
null,
List.of(
new VersionRegexRepoFilter(
- "org[.]sitemesh.*", ".*", ".*-SNAPSHOT"
+ "org[.]sitemesh.*", ".*", ".*"
Review Comment:
Is the M1 and actual release? I don't think this code should change since the below filter will still filter to only snapshots
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org