Skip to content

fix(iroh): Implement Clone for StaticProvider discovery #3108

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

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

flub
Copy link
Contributor

@flub flub commented Jan 8, 2025

Description

This is not very useful without Clone, it already is Arc<RwLock> on the inside so this clearly was intended. Write a test demonstrating why this is needed.

Breaking Changes

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

@flub flub requested a review from rklaehn January 8, 2025 17:38
Copy link

github-actions bot commented Jan 8, 2025

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3108/docs/iroh/

Last updated: 2025-01-09T08:56:57Z

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 flub force-pushed the flub/discovery-arc branch from bf23bbe to 4d28dfa Compare January 8, 2025 17:44
This is not very useful without Clone, it already is Arc<RwLock<T>> on
the inside to this clearly was intended.  Write a test demonstrating
why this is needed.
@flub flub force-pushed the flub/discovery-clone branch from 5af139d to 07de2c8 Compare January 8, 2025 17:45
@flub flub requested a review from a team January 8, 2025 17:55
Copy link

github-actions bot commented Jan 8, 2025

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 72c5cac

@flub flub requested a review from dignifiedquire January 9, 2025 11:05
@flub flub self-assigned this Jan 9, 2025
Base automatically changed from flub/discovery-arc to main January 13, 2025 10:35
@flub flub added this pull request to the merge queue Jan 13, 2025
Merged via the queue into main with commit 65cf688 Jan 13, 2025
26 checks passed
@flub flub deleted the flub/discovery-clone branch January 13, 2025 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants