Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.4k
udp: use libuv API to get file descriptor#6908
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
bnoordhuis commented May 21, 2016
LGTM |
jasnell commented May 21, 2016
LGTM... would be ideal if there was some way to actually test this tho so we don't get regressions later |
cjihrig commented May 21, 2016
LGTM |
saghul commented May 21, 2016
@jasnell I'm open to suggestions, but how do we test that the way to obtain that value has changed in C++ ? FWIW, before this change Node was relying on libuv internals, which can be seen as "broken" already, because we could've changed it without notice. |
jasnell commented May 22, 2016
;-) I didn't say I had any good suggestions for a test ;-) I'm fine with this landing as is, but if someone can come up with a way of testing then +1 |
Refs: nodejs#6838 PR-URL: nodejs#6908 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: nodejs#6838 PR-URL: nodejs#6908 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #6838 PR-URL: #6908 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins commented Jun 3, 2016
@saghul is it safe to assume that this will be neccessary if we want to backport libuv 1.9? Not entirely sure if this should land or not either way |
saghul commented Jun 3, 2016
I'd land it because it poses 0 risk and it makes process.std*._handle.fd
|
Refs: nodejs#6838 PR-URL: nodejs#6908 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #6838 PR-URL: #6908 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #6838 PR-URL: #6908 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #6838 PR-URL: #6908 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #6838 PR-URL: #6908 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #6838 PR-URL: #6908 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #6838 (comment)