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.

Sign up for GitHub

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

Jump to bottom

feat(tcp): make TCP_NODELAY the default #4916

Closed
wants to merge 0 commits into from
Closed

feat(tcp): make TCP_NODELAY the default #4916

wants to merge 0 commits into from

Conversation

shubhexists
Copy link

@shubhexists shubhexists commented Nov 23, 2023 •
edited by thomaseizinger
Loading

Description

Fixes: #4890.

Notes

Added no delay to be Some(true) by default whenever the new method is called.

  • I have made corresponding changes to the documentation
  • I have performed a self-review of my own code

@shubhexists shubhexists changed the title make TCP_NODELAY the default feat(tcp): make TCP_NODELAY the default Nov 23, 2023
mxinden
mxinden reviewed Nov 27, 2023
View reviewed changes
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!

Mind adding a changelog entry for the next libp2p-tcp minor release?

transports/tcp/src/lib.rs Outdated Show resolved Hide resolved
@mxinden mxinden added this to the v0.54.0 milestone Nov 27, 2023
@shubhexists
Copy link
Author

shubhexists commented Nov 27, 2023 •
edited
Loading

Hi, apologies for the mistake. Fixed the function comment and added to the PR to the Changelog.md file
I read the issue wrong. I'll fix it as required
Edit - Also checked the box in the 1st comment.

thomaseizinger
thomaseizinger requested changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

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

@mxinden In my eyes, this is a breaking change and shouldn't be released as a patch-release.

transports/tcp/CHANGELOG.md Outdated Show resolved Hide resolved
@shubhexists
Copy link
Author

Updated the Changelog and Cago.toml as asked @thomaseizinger

@thomaseizinger thomaseizinger marked this pull request as draft November 30, 2023 07:23
@thomaseizinger
Copy link
Contributor

Thanks! This is ready but we delay breaking changes for a while to batch them up. I marked it as draft so it doesn't get merged accidentally.

Can you deal with the CI error?

jxs
jxs reviewed Jun 13, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@jxs jxs closed this Jun 13, 2024
@jxs jxs mentioned this pull request Jun 13, 2024
Merged
2 tasks
@jxs jxs removed this from the v0.54.0 milestone Jun 13, 2024
mergify bot pushed a commit that referenced this pull request Jun 13, 2024
@jxs
feat(tcp): make TCP_NODELAY the default
a35e269 
Superseeds #4916.
Fixes: #4890.

Pull-Request: #5469.
@jxs
Copy link
Member

jxs commented Jun 13, 2024

sorry @shubhexists could not make the changes to your branch as you PR'd from master opened #5469 preserving your commits :)
Thanks!

TimTinkers pushed a commit to unattended-backpack/rust-libp2p that referenced this pull request Sep 14, 2024
@jxs @TimTinkers
feat(tcp): make TCP_NODELAY the default
4acbd80 
Superseeds libp2p#4916.
Fixes: libp2p#4890.

Pull-Request: libp2p#5469.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mxinden mxinden mxinden left review comments

@thomaseizinger thomaseizinger thomaseizinger requested changes

@jxs jxs jxs left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat(tcp): make TCP_NODELAY the default
4 participants
@shubhexists @thomaseizinger @jxs @mxinden