Skip to content

Conversation

@karthik2804
Copy link
Collaborator

This PR fixes an issue discovered by @kate-goldenring. The default template without a router failed to compile because it did not have any regex, and the return of precompile was a string rather than an object. This PR gives the precompile function a consistent return type.

Copy link
Collaborator

@tschneidereit tschneidereit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test for this by any chance? Other than that, LGTM!

@karthik2804
Copy link
Collaborator Author

karthik2804 commented Nov 4, 2025

@tschneidereit I added a test but now realize it is going to fail until we have a new release as I added it to the template test . Do we want to do that or just add a new test that points at the latest build of the tools?

The more I think the more I feel like I should do the later as well as testing the templates can be run successfully.

@tschneidereit
Copy link
Collaborator

The more I think the more I feel like I should do the later as well as testing the templates can be run successfully.

Yeah, I think that's what we should do: in general we should be able to test changes before releasing them, instead of having failing tests that can only be fixed with a release. If we're doing the latter, we always run the risk of having some other issue causing the test to fail, which is hidden until we do the release.

@karthik2804 karthik2804 force-pushed the fix_precompile branch 6 times, most recently from 4a66bf5 to b771909 Compare November 4, 2025 13:10
Signed-off-by: Karthik Ganeshram <[email protected]>
@karthik2804 karthik2804 merged commit a8b4d2d into spinframework:main Nov 5, 2025
11 checks passed
@karthik2804 karthik2804 deleted the fix_precompile branch November 5, 2025 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants