Skip to content

feat(redirect): add include_path (and trim_prefix) for wildcard redirect support#11

Closed
vdice wants to merge 1 commit intospinframework:mainfrom
vdice:feat/wildcard-redirects
Closed

feat(redirect): add include_path (and trim_prefix) for wildcard redirect support#11
vdice wants to merge 1 commit intospinframework:mainfrom
vdice:feat/wildcard-redirects

Conversation

@vdice
Copy link
Copy Markdown
Contributor

@vdice vdice commented Apr 7, 2025

Ref #4

Thanks to @rajatjindal and @kingdonb for their reference implementation(s) mentioned in #4

If we're in favor of this general direction, I can add unit test(s). And a mention in the README edit: added.

I lumped in the 'trim prefix' functionality both because it is immediately useful for some work I have in flight 😅 and it seems like it would be a natural bit of functionality that users might want after including the request path. However, if this aspect doesn't land with reviewers and needs a rethink -- or we'd just prefer it in a separate change, I can defer to a follow-up.

@itowlson
Copy link
Copy Markdown
Contributor

itowlson commented Apr 7, 2025

I realise I'm not the right reviewer anyway, but I think the README bit would be helpful to me to understand the usage and behaviour here (certainly no objection to doing both bits in one PR).

…ect support

Signed-off-by: Vaughn Dice <vaughn.dice@fermyon.com>
@vdice vdice force-pushed the feat/wildcard-redirects branch from fb020e6 to 1c51c0e Compare April 8, 2025 00:30
@vdice
Copy link
Copy Markdown
Contributor Author

vdice commented Apr 8, 2025

@itowlson thank you, review/feedback much appreciated. README.md addition added.

Ran into a hiccup attempting to run unit tests... gonna pair with resident (tiny)go expert @adamreese tomorrow...

@itowlson
Copy link
Copy Markdown
Contributor

itowlson commented Apr 8, 2025

Thank you @vdice, the README changes made it click, especially the examples!

@vdice
Copy link
Copy Markdown
Contributor Author

vdice commented Apr 8, 2025

Superseded by #12; @adamreese helped bump the sdk and perform some re-org to enable adding unit tests.

@vdice vdice closed this Apr 8, 2025
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