Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Use class from new template engine module #654

Merged

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Nov 3, 2024

The template engine code is in its own module now, so we need to add it as a dependency and update the use statement. Going with API since that feels like it takes precedence over DEP and updating a use statement feels like a API commit to me, but I'm happy to change the prefix if the reviewer wants me to.

I couldn't think of a clean way to easily abstract this out, given it intentionally uses the same configuration that template partial caching uses, and that is a concept specific to ss templates that can't be abstracted to be for generic template engines.

I think it's sensible to leave this as a strict dependency for now given we have strong coupling to this template engine for all of the CMS templates anyway. If we ever make a move to decouple from the module, we can take another look here at that time - that would need to happen in a future major release anyway since it would mean (at a minimum) moving our CMS templates out of their current modules.

CI for this can't go green until silverstripe/silverstripe-framework#11451 has been merged

Issue

@emteknetnz emteknetnz merged commit 4d3042e into silverstripe:3 Nov 5, 2024
6 of 9 checks passed
@emteknetnz emteknetnz deleted the pulls/3/template-engine-moved branch November 5, 2024 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants