Skip to content

Conversation

@thomaseizinger
Copy link
Contributor

@thomaseizinger thomaseizinger commented Dec 14, 2022

Description

Previously, we used to buffer events separately and emit actions directly. That is unnecessary. We can have a single place where we return from the poll loop and shove all actions into the same buffer.

Notes

I originally created this because I thought it would fix an issue where I needed to supply type hints as part of implementing #3240. That didn't turn out to be true but I still think that this change is a slight improvement. Feel free to close if you disagree :)

Links to any relevant issues

Open Questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@thomaseizinger
Copy link
Contributor Author

I originally created this because I thought it would fix an issue where I needed to supply type hints as part of implementing #3240. That didn't turn out to be true but I still think that this change is a slight improvement. Feel free to close if you disagree :)

Turns out the issue I am running in might be a rustc bug: rust-lang/rust#105680

Copy link
Member

@elenaf9 elenaf9 left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@mergify mergify bot merged commit f6f4296 into master Dec 15, 2022
@thomaseizinger thomaseizinger deleted the refactor-autonat/buffer-entire-actions branch December 16, 2022 20:40
umgefahren pushed a commit to umgefahren/rust-libp2p that referenced this pull request Mar 8, 2024
Previously, we used to buffer events separately and emit actions directly. That is unnecessary. We can have a single place where we return from the `poll` loop and shove all actions into the same buffer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants