search

quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.76k stars 380 forks source link

config: add ServerConfig::with_transport_config() builder method #1314

Closed djc closed 2 years ago

djc commented 2 years ago

Fixes #1301.

djc commented 2 years ago

(Failure is unrelated, fixed in #1316.)

BastiDood commented 2 years ago

Hello there! I'm surprised that the with_transport_config setters take an Arc<TransportConfig> instead of a plain TransportConfig. Shouldn't it be the latter instead since TransportConfig is a concrete type rather than a dyn object (in which case the Arc is actually necessary)?

Although it is honest to its internal representation, shouldn't the user code not have to worry about wrapping it in an Arc first?

djc commented 2 years ago

I would say this is actually useful. For example, if you have multiple endpoints with the same TransportConfig, you can now share the same TransportConfig. Additionally it can often be useful to have the caller supply the exact type you need, rather than the callee taking care of some conversion (arguably my example is just one specific example of that same principle).

BastiDood commented 2 years ago

If you have multiple endpoints with the same TransportConfig, you can now share the same TransportConfig.

Fair point. I'm convinced. 🎉

djc commented 2 years ago

Fixed the commit message.