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

Vary middleware #21

Merged
merged 11 commits into from
Jun 20, 2024
Merged

Vary middleware #21

merged 11 commits into from
Jun 20, 2024

Conversation

imbolc
Copy link
Contributor

@imbolc imbolc commented May 14, 2024

Here's how it looks (just a draft yet). Let me know if you're ok with the approach before I finish it.

Also I've noticed you implemented tower middleware in the guard, is there any benefits compared to axum::middleware::from_fn. I can't imagine how one would use the crate without axum, so depending on it instead of axum-core seems ok, isn't it?

@imbolc imbolc marked this pull request as draft June 2, 2024 11:04
@imbolc
Copy link
Contributor Author

imbolc commented Jun 2, 2024

@robertwayne haven't had a chance to look at it yet?

@robertwayne
Copy link
Owner

Woops, sorry about that - it got lost in my sea of notifications. I'll look over this later today!

@imbolc
Copy link
Contributor Author

imbolc commented Jun 3, 2024

Sure, no hurry :)

@robertwayne
Copy link
Owner

Yeah, I'm good with this approach. It's about what I envisioned when you originally brought up channels. You can finish This should live behind a feature, for sure.

Also I've noticed you implemented tower middleware in the guard, is there any benefits compared to axum::middleware::from_fn. I can't imagine how one would use the crate without axum, so depending on it instead of axum-core seems ok, isn't it?

Yeah, it certainly could have used the axum from_fn method. I just chose to go with the most flexible (and lowest level) option available after initially reading through the axum middleware documentation.

@imbolc imbolc marked this pull request as ready for review June 16, 2024 06:48
@imbolc imbolc changed the title Vary middleware draft Vary middleware Jun 18, 2024
@imbolc
Copy link
Contributor Author

imbolc commented Jun 18, 2024

@robertwayne just making sure you don't miss it, no hurry

@robertwayne
Copy link
Owner

Thanks for the ping (and the work on this)!

This looks great to me; tests, docs, and example. Thank you again!

@robertwayne robertwayne merged commit 1c4fa34 into robertwayne:main Jun 20, 2024
2 checks passed
@imbolc imbolc deleted the vary-middleware branch June 21, 2024 02:59
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.

None yet

2 participants