feat(iroh): implement Discovery for Arc'ed Discovery types #3107
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3107/docs/iroh/ Last updated: 2025-01-10T10:14:07Z |
flub
force-pushed
the
flub/discovery-arc
branch
from
5af139d to
bf23bbe
Compare
Nothing in the trait stops it from working for discovery structs which are Arc'ed. Not all discovery types are in control of the users so this is helpful. I stumbled upon this because StaticProvider is not Clone.
flub
force-pushed
the
flub/discovery-arc
branch
from
bf23bbe to
4d28dfa
Compare
iroh/src/discovery.rs
Outdated
| @@ -725,6 +728,21 @@ mod tests { | |||
| .expect("time drift") | |||
| .as_micros() as u64 | |||
| } | |||
|
|
|||
| #[tokio::test] | |||
| async fn test_arc_discovery() -> TestResult { | |||
There was a problem hiding this comment.
maybe use something like static_assert instead?
There was a problem hiding this comment.
I'm unsure what you have in mind. Could you elaborate a bit more?
There was a problem hiding this comment.
using https://crates.io/crates/static_assertions you can just check for trait bounds, instead of doing this manual adding
There was a problem hiding this comment.
I guess that has assert_impl_all which I could use. But then, how? Adding it at build-time would just put it right below the impl... which seems a bit odd, because it's implemented right there. Plus it would add a new build-time dependency.
I can use it in the test I guess, but again a new dependency seems like a lot.
I added this test mostly to assure myself I did the right thing. But talking about it here seems to have little value? Maybe it should just be removed instead?
There was a problem hiding this comment.
Maybe it should just be removed instead?
maybe, not sure testing this makes some sense, but also it's a static thing, so I don't think there is much value in the test
Description
Nothing in the trait stops it from working for discovery structs which
are Arc'ed. Not all discovery types are in control of the users so
this is helpful.
I stumbled upon this because StaticProvider is not Clone.
Breaking Changes
Notes & open questions
Change checklist