Skip to content
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

fix(iroh-gossip): clarify docs and semantics of gossip joined event #2597

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

Frando
Copy link
Member

@Frando Frando commented Aug 6, 2024

Description

Improves documentation around the GossipEvent::Joined event: It is only emitted once at the beginning of the stream, and the event will not be emitted when awaiting GossipReceiver::joined.

Also makes sure that the event is actually only emitted once per intent (it potentially could have been emitted multiple times before if the neighbor count first got down to 0 and then up again for GossipTopics subscribing inbetween).

Breaking Changes

Notes & open questions

Inspired by the discussion in deltachat/deltachat-core-rust#5860

Change checklist

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

@Frando Frando changed the title fix: clarify docs and semantics of gossip joined event fix(iroh-gossip): clarify docs and semantics of gossip joined event Aug 6, 2024
Copy link

github-actions bot commented Aug 6, 2024

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

Last updated: 2024-08-12T10:47:40Z

@Frando Frando marked this pull request as ready for review August 7, 2024 08:09
.await?
.context("Gossip receiver closed before Joined event was received.")?
{
Event::Gossip(GossipEvent::Joined(_)) => {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you throwing the initial neighbours here away?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are added to self.neighbors in poll_next already.

@dignifiedquire dignifiedquire added this pull request to the merge queue Aug 12, 2024
Merged via the queue into main with commit 5d98a5c Aug 12, 2024
28 checks passed
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