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

feat(tcp): make TCP_NODELAY the default #4916

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

shubhexists
Copy link

@shubhexists shubhexists commented Nov 23, 2023

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

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.

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 and others added 2 commits November 28, 2023 18:01
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
@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?

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.

feat(tcp): make TCP_NODELAY the default
3 participants