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

core/transport: Remove various Sync bounds #2667

Merged
merged 3 commits into from
May 29, 2022

Conversation

thomaseizinger
Copy link
Contributor

@thomaseizinger thomaseizinger commented May 23, 2022

Description

With Transport becoming non-Clone and having &mut self receivers,
the Sync requirement no longer makes any sense and we can thus
remove it.

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

With `Transport` becoming non-Clone and having `&mut` self receivers,
the `Sync` requirement no longer makes any sense and we can thus
remove it.
@thomaseizinger thomaseizinger changed the title Remove various Sync bounds core/transport: Remove various Sync bounds May 23, 2022
Copy link
Contributor

@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.

Nice! I actually stumbled over that recently because it was that reason that you could not call boxed directly on the tcp transport. So this will also make #2652 (comment) easier.

@thomaseizinger
Copy link
Contributor Author

Was about to merge this but will hold back until 0.45.0 is out.

elenaf9 added a commit to elenaf9/rust-libp2p that referenced this pull request May 29, 2022
With PR libp2p#2667 the `Sync` trait bound for transport::Boxed is removed.
If a tcp transport should be polled as a stream we can now do this via
`TcpTransport::new(..)::boxed` and do not need a separate impl of
`Stream` for it.
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Thanks!

I am in favor of including this in v0.45.0.

@thomaseizinger
Copy link
Contributor Author

Thanks!

I am in favor of including this in v0.45.0.

Okay, up to you!

@mxinden mxinden merged commit 4aa84bf into libp2p:master May 29, 2022
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