Merge sitemesh3 grails plugin into grails-gsp #562
Conversation
…provided by GroovyPagesGrailsPlugin.
… grails and grails once sitemesh has been decoupled.
|
|
||
| implementation platform("org.grails:grails-bom:$grailsVersion") | ||
|
|
||
| api project(':grails-taglib'), { // GrailsTagLibClass |
There was a problem hiding this comment.
@matrei jsp isn't even included here now. can you explain why?
There was a problem hiding this comment.
Why was it included before? From what I could find, there is no dependency on that library in this module.
There was a problem hiding this comment.
I assume it was so JSPs could use tags. I'm hesitant to change this one unless we write some tests to ensure it doesn't break anything.
There was a problem hiding this comment.
I guess what I'm saying is if we don't know why it was included, and we know it's working, I don't think now is the time to be removing it. We should leave it for now and fix that on a second pass. Otherwise we could unintentionally break downstream projects.
There was a problem hiding this comment.
I'm planning to add apps testing JSP (with JSTL) and GSP (with taglibs) in the examples folder. Then we will get clarity about the dependencies.
There was a problem hiding this comment.
Thank you @matrei That's my main concern!
grails-web-jsp/src/main/groovy/org/grails/gsp/jsp/JspTagImpl.groovy
Outdated
Show resolved
Hide resolved
This commit removes (comments out) the excluded transitive dependencies. They were used to visualize the true direct dependencies of these modules. The commented out exclusions are kept in the build files to assist with further decoupling going forward.
This reverts commit 56513bb.
After the revert in the last commit FastStringWriter is also used from `grails-encoder`.
| exclude group: 'org.springframework', module: 'spring-core' | ||
| */ | ||
| } | ||
| implementation project(':grails-web-gsp-taglib'), { // RenderTagLib |
There was a problem hiding this comment.
Doesn't this still need to be API?
Since the project now relies on the BOM for the `groovyVersion` instead of specifying it explicitly, the version must be resolved dynamically to ensure guide links point to the correct Groovy API documentation.
Add `grails-plugin-codecs` as it is required at runtime by `grails-plugin-sitemesh3` for the `CodecLookup` bean.
To get rid of warning message.
Add `grails-plugin-codecs` as it is required at runtime by `grails-plugin-gsp` for the `CodecLookup` bean.
Add `grails-plugin-rest` as the app uses `respond()`.
|
Per grails/grails-core#13678 , I'm going to go ahead and merge this and let @matrei add the test scenarios we've previously agreed on. |
# Conflicts: # gradle.properties
grails/grails-core#13552
Merges the sitemesh3 grails plugin written by @codeconsole that replaced the sitemesh2 plugin.
When comparing this PR, look at the last commit only to make it easier to check.
A significant amount of rework went into this PR: