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

*: Add IfWatcher::poll_if_event instead of IfWatcher::poll_next #25

Merged
merged 4 commits into from
Aug 19, 2022

Conversation

elenaf9
Copy link
Contributor

@elenaf9 elenaf9 commented Aug 13, 2022

Since IfWatcher is Unpin we can make poll_next take &mut self instead of self: Pin<&mut Self> and implement Stream by using Pin::into_inner.

Proposed by @thomaseizinger in libp2p/rust-libp2p#2813 (comment).
@thomaseizinger did I understand it correctly that this was the idea?


Also renamed poll_next to poll_if_event in e56e686 to avoid conflicting names with <IfWatcher as Stream>::poll_next. I don't feel strongly about that, happy to drop that commit it if folks prefer to stick with poll_next.

Copy link

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Yep, that is what I was thinking!

@thomaseizinger
Copy link

PR title has a typo with &self instead of &mut self.

@elenaf9 elenaf9 changed the title *: make IfWatcher::poll_next take &self *: make IfWatcher::poll_next take &mut self Aug 14, 2022
@mxinden
Copy link
Owner

mxinden commented Aug 16, 2022

I am fine with this. Can you add a changelog entry @elenaf9?

@elenaf9 elenaf9 changed the title *: make IfWatcher::poll_next take &mut self *: Add IfWatcher::poll_if_event instead of IfWatcher::poll_next Aug 18, 2022
@mxinden mxinden merged commit d876c4c into mxinden:master Aug 19, 2022
@elenaf9 elenaf9 deleted the hide-pinning branch August 19, 2022 07:10
@elenaf9
Copy link
Contributor Author

elenaf9 commented Aug 19, 2022

With this PR merged, all changes from my side required for simplifying the IfWatcher integration in rust-libp2p (libp2p/rust-libp2p#2813) are done now. Could you cut a new release @mxinden? :)

@mxinden
Copy link
Owner

mxinden commented Aug 20, 2022

@elenaf9 cool. Done via #26.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants