Implement QUIC traits with Quinn #4
Conversation
|
I've merged #3. This looks really cool! I think we could have the integrations be in a separate crate, especially since this is creating new structs to implement the traits on anyways. We could create an |
|
Yeah that makes more sense. Thanks! |
stammw
force-pushed
the
quinn
branch
2 times, most recently
from
ab5b4c5 to
8ec3938
Compare
|
So I've got this sorted out with workspaces. I hope this what you meant ? I also got rid of |
|
Awesome, yes this is what I had in mind! So it looks like we can't require Rust 1.39 as the minimum version (I just picked that since that's currently in hyper). Do you know what version Quinn does require? |
|
I don't know if there is a particular rust version target for |
|
Yea I'm not super worried about needing a newer version, I just like to have it recorded in CI, so we know whenever it needs to change. We can update the value in CI in this PR to make it happy with Quinn. |
|
Okay, so 1.40 is enough. |
h3-quinn/src/lib.rs
Outdated
|
|
||
| use h3::quic; | ||
|
|
||
| struct QuinnConnection<S: Session> { |
There was a problem hiding this comment.
shouldn't it be public, so that it can then be passed to h3::server::Connection::handshake?
There was a problem hiding this comment.
Yes that should! Plus a new(quinn::Connection) function should be handy here. Thanks!
There was a problem hiding this comment.
also NIT: since this is already under the quinn-h3 namespace, it could be named just Connection and then let the importer rename if conflicting
There was a problem hiding this comment.
Do you mean QuinnConnection should be named Connection ?
There was a problem hiding this comment.
yeah, and then if/when naming conflicts occur, you can import with use quinn_h3::Connection as QuinnConnection
There was a problem hiding this comment.
That's actually a lot better, I thought I would need those prefixes, but no.
stammw
force-pushed
the
quinn
branch
2 times, most recently
from
449f552 to
aabf91a
Compare
|
So we're fine to merge? Should I squash the commits as the green github button suggests ? |
|
Squash or rebase, if the commits make sense separated. If the commits are just addressing review feedback, then they should probably be squashed. |
Here is a completely untested QUIC traits implementation with
quinn.It's based on #3 and contains some change suggestions to it. The draft status should disappear along with them.