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

Fix using template parts via render-template #88

Merged

Conversation

patrickbkr
Copy link
Member

The @*CRO-TEMPLATE-PART-PROVIDERS dynvar was only set in the template multis. Moving it to render-internal should cover all needed code paths.

@japhb
Copy link

japhb commented Mar 6, 2024

We've got a compatibility problem in our CI -- it looks like it may be failing to install dependencies on 2021.03, and of course to save compute time GH just cancels all the test jobs when any one fails.

However I reran only ubuntu-latest/latest to work around the incompatibility and that gave:

https://github.com/croservices/cro-webapp/actions/runs/8179001098/job/22368196868

That looks actually related to the content of the PR. @patrickbkr Can you take a look at that one?

@patrickbkr
Copy link
Member Author

I believe render-template should behave the same as template, just as the documentation states.

It's an interesting problem. In the tests render-template is used outside of a route block. router-plugin-get-configs throws when called outside a route or middleware block.

To make render-template (and template as well) usable outside route blocks, I could either make the setting of @*CRO-TEMPLATE-PART-PROVIDERS conditional duplicating the same checks. Or I catch the respective exception and ignore it. Ignoring the exception seems like the more maintainable approach, but uses an exception in a non-error code path. (Which is generally considered bad practice.) I still think that's the best approach. Other ideas?

The `@*CRO-TEMPLATE-PART-PROVIDERS` dynvar was only set in the `template`
multis. Moving it to `render-internal` should cover all needed code paths.
@patrickbkr patrickbkr force-pushed the render-template-template-parts branch from 4260ce1 to 68905c3 Compare March 7, 2024 20:07
@patrickbkr
Copy link
Member Author

I went with the CATCH approach. All tests should pass now.

@patrickbkr patrickbkr merged commit 62e0a72 into croservices:master Nov 19, 2024
0 of 4 checks passed
@patrickbkr patrickbkr deleted the render-template-template-parts branch November 19, 2024 21:35
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