-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
Add routerify integration #487
Conversation
This can also be generalized as a hyper integration since there is not any routerify specific code here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the implementation does not require routerify at all, let's call the crate askama_hyper
and rename all the bits in other Askama crates to hyper
. Then rename the test file to routerify.rs
(and please just merge support.rs
into it.)
readme = "README.md" | ||
edition = "2018" | ||
|
||
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please get rid of this.
buf.writeln(&format!("::askama_routerify::respond(&self, {:?})", ext))?; | ||
buf.writeln("}")?; | ||
buf.writeln("}") | ||
} | ||
// Writes header for the `impl` for `TraitFromPathName` or `Template` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an empty line here below the new method?
[dependencies] | ||
askama = { version = "0.10.6", path = "../askama", default-features = false, features = ["with-routerify", "mime", "mime_guess"] } | ||
hyper = "0.14" | ||
routerify = "2.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, move routerify
to be a dev dep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we turn this to askama_hyper I think we can drop it completely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me!
Did most of the stuff but the problem is we need the client feature of hyper on testing. It looks like testing only features is a open issue on cargo repo. So I don't know how to test the hyper code. Any idea? |
Sorry, not sure what you mean, can you give me more context for what you're trying to do? |
We test the http framework integrations and testing actix one is kinda hard since we need some kind of client or optionally activate the client feature on tests (i dont think this one is possible). |
You can just repeat the dependency in the dev-dependencies with the extra feature added? |
closes #484