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

Revert "dgram: implicit binds should be exclusive" #279

Merged
merged 1 commit into from
Jan 10, 2015

Conversation

bnoordhuis
Copy link
Member

This reverts commit a32b92d.

Reverted for breaking the parallel/test-cluster-dgram-2 test on all
platforms.

R=@piscisaureus @sam-github

@bnoordhuis
Copy link
Member Author

@cjihrig
Copy link
Contributor

cjihrig commented Jan 10, 2015

LGTM. Might be worth fixing instead of just reverting if it is a quick fix.

@rvagg
Copy link
Member

rvagg commented Jan 10, 2015

Is this going to land us with a minor/subtle incompatibility with joyent/node?

@cjihrig
Copy link
Contributor

cjihrig commented Jan 10, 2015

Yes, but the same PR is open on joyent/node and should be in 0.12.

@rvagg
Copy link
Member

rvagg commented Jan 10, 2015

In that case LGTM

@cjihrig
Copy link
Contributor

cjihrig commented Jan 10, 2015

Let me clarify - the original PR to add this feature is open on joyent/node. The revert is only on io.js, but I assume the revert would be needed there as well.

This reverts commit a32b92d.

Reverted for breaking the parallel/test-cluster-dgram-2 test on all
platforms.

PR-URL: nodejs#279
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@bnoordhuis bnoordhuis merged commit 0526d83 into nodejs:v1.x Jan 10, 2015
@bnoordhuis bnoordhuis deleted the revert-a32b92d branch January 10, 2015 18:19
@bnoordhuis
Copy link
Member Author

I didn't spent too much time investigating why the test fails, that's for Sam and Bert to figure out. I just ran git bisect to find the offending commit.

@sam-github
Copy link
Contributor

I'll figure out and fix.​

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.

4 participants