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

Do not closeFd within sendFd #271

Merged
merged 3 commits into from
Dec 15, 2017
Merged

Conversation

eborden
Copy link
Collaborator

@eborden eborden commented Dec 15, 2017

It is common to close a file descriptor after sending it, however it is
not a required part of the work flow. There are uses for not closing a
file descriptor. Closing the file descriptor is too high level of a
decision.

Fix for #150

@eborden eborden self-assigned this Dec 15, 2017
@kazu-yamamoto
Copy link
Collaborator

We should use return instead of pure. I will take care of this.

eborden and others added 3 commits December 15, 2017 19:13
It is common to close a file descriptor after sending it, however it is
not a required part of the work flow. There are uses for not closing a
file descriptor. Closing the file descriptor is too high level of a
decision.
@kazu-yamamoto kazu-yamamoto force-pushed the dont-close-fd-in-send-fd branch from 34bdbc4 to 76d4802 Compare December 15, 2017 10:13
Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

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

LGTM.

@kazu-yamamoto kazu-yamamoto merged commit 76d4802 into master Dec 15, 2017
kazu-yamamoto added a commit that referenced this pull request Dec 15, 2017
@kazu-yamamoto kazu-yamamoto deleted the dont-close-fd-in-send-fd branch December 15, 2017 10:15
@kazu-yamamoto
Copy link
Collaborator

Merged.

@glguy
Copy link
Member

glguy commented Mar 2, 2018

This seems like a huge change to slip into the package without so much as a minor version bump!

It was added in the transition from 2.6.3.3 to 2.6.3.4

@kazu-yamamoto
Copy link
Collaborator

@glguy Sorry for your inconvenience.
We believed that this was a bug fix.
What should we do?

@glguy
Copy link
Member

glguy commented Mar 5, 2018

This probably needs to be a major version change, it changes the behavior of an existing function in a nonbackwards-compatible way. Code that uses this function will need to be modified to work after this change. The CPP macros can't even distinguish a change in the 4th component of a version number.

https://pvp.haskell.org

@kazu-yamamoto
Copy link
Collaborator

@glguy

You suggest:

  • 2.6.3.5 with this reverted should be released ASAP
  • We should set not-recommended flag to 2.6.3.4
  • this change should be done in 2.7

Am I understanding correctly?

This was referenced Mar 9, 2018
kazu-yamamoto added a commit to kazu-yamamoto/network that referenced this pull request Mar 12, 2018
This reverts commit e035223, reversing
changes made to 515c323.
@eborden eborden mentioned this pull request Apr 28, 2018
17 tasks
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