Skip to content

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

Merged
mxinden merged 4 commits into
libp2p:masterfrom
elenaf9:hide-pinning
Aug 19, 2022
Merged

*: Add IfWatcher::poll_if_event instead of IfWatcher::poll_next#25
mxinden merged 4 commits into
libp2p:masterfrom
elenaf9:hide-pinning

Conversation

@elenaf9

@elenaf9 elenaf9 commented Aug 13, 2022

Copy link
Copy Markdown
Member

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.

@thomaseizinger thomaseizinger left a comment

Copy link
Copy Markdown

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
Copy Markdown

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

mxinden commented Aug 16, 2022

Copy link
Copy Markdown
Member

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 libp2p:master Aug 19, 2022
@elenaf9
elenaf9 deleted the hide-pinning branch August 19, 2022 07:10
@elenaf9

elenaf9 commented Aug 19, 2022

Copy link
Copy Markdown
Member Author

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

mxinden commented Aug 20, 2022

Copy link
Copy Markdown
Member

@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