Skip to content

Conversation

@danbev
Copy link
Contributor

I cannot find any usage of uv in the header and think that it can be
removed.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

I cannot find any usage of uv in the header and think that it can be removed.
@nodejs-github-botnodejs-github-bot added async_wrap c++ Issues and PRs that require attention from people who are familiar with C++. labels May 11, 2017
@danbev
Copy link
ContributorAuthor

Copy link
Contributor

@thefourtheyethefourtheye left a comment

Choose a reason for hiding this comment

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

LGTM if the CI is happy.

@danbev
Copy link
ContributorAuthor

danbev commented May 11, 2017

test/freebsd failure does not look related https://ci.nodejs.org/job/node-test-commit-freebsd/nodes=freebsd11-x64/8965/console:
not ok 735 parallel/test-net-connect-local-error --- duration_ms: 0.356 severity: fail stack: |- Mismatched onError function calls. Expected 1, actual 0. at Object.<anonymous> (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/parallel/test-net-connect-local-error.js:15:27) at Module._compile (module.js:569:30) at Object.Module._extensions..js (module.js:580:10) at Module.load (module.js:503:32) at tryModuleLoad (module.js:466:12) at Function.Module._load (module.js:458:3) at Function.Module.runMain (module.js:605:10) at startup (bootstrap_node.js:144:16) at bootstrap_node.js:561:3 ...not ok 736 parallel/test-net-connect-options-allowhalfopen --- duration_ms: 0.535 severity: fail stack: |- Mismatched serverOnConnection function calls. Expected 6, actual 7. at forAllClients (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/parallel/test-net-connect-options-allowhalfopen.js:44:38) at Object.<anonymous> (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/parallel/test-net-connect-options-allowhalfopen.js:55:21) at Module._compile (module.js:569:30) at Object.Module._extensions..js (module.js:580:10) at Module.load (module.js:503:32) at tryModuleLoad (module.js:466:12) at Function.Module._load (module.js:458:3) at Function.Module.runMain (module.js:605:10) at startup (bootstrap_node.js:144:16) Mismatched <anonymous> function calls. Expected 1, actual 0. at Server.serverOnConnection (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/parallel/test-net-connect-options-allowhalfopen.js:61:30) at Server.<anonymous> (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/common/index.js:504:15) at emitOne (events.js:115:13) at Server.emit (events.js:210:7) at TCP.onconnection (net.js:1551:8) Server starts at localhost:18418 1 'connection' emitted on server Server has sent 1 FIN 2 'connection' emitted on server Server has sent 2 FIN 3 'connection' emitted on server Server has sent 3 FIN 4 'connection' emitted on server Server has sent 4 FIN 5 'connection' emitted on server Server has sent 5 FIN 6 'connection' emitted on server Server has sent 6 FIN 7 'connection' emitted on server Server has sent 7 FIN 'connect' emitted on Client 0 'connect' emitted on Client 1 'connect' emitted on Client 2 'connect' emitted on Client 3 'connect' emitted on Client 4 'connect' emitted on Client 5 Server recieved FIN sent by No. undefined No. 0 client received FIN No. 0 client sent FIN, 1 have been sent No. 1 client received FIN No. 1 client sent FIN, 2 have been sent No. 2 client received FIN No. 2 client sent FIN, 3 have been sent No. 3 client received FIN No. 3 client sent FIN, 4 have been sent No. 4 client received FIN No. 4 client sent FIN, 5 have been sent No. 5 client received FIN No. 5 client sent FIN, 6 have been sent 2 server connection is started by client No. 0 3 server connection is started by client No. 1 4 server connection is started by client No. 2 5 server connection is started by client No. 3 6 server connection is started by client No. 4 7 server connection is started by client No. 5 No. 5 connection has been closed by both sides, 1 clients have closed No. 4 connection has been closed by both sides, 2 clients have closed No. 3 connection has been closed by both sides, 3 clients have closed No. 2 connection has been closed by both sides, 4 clients have closed No. 1 connection has been closed by both sides, 5 clients have closed No. 0 connection has been closed by both sides, 6 clients have closed Server recieved FIN sent by No. 0 Server recieved FIN sent by No. 1 Server recieved FIN sent by No. 2 Server recieved FIN sent by No. 3 Server recieved FIN sent by No. 4 Server recieved FIN sent by No. 5 No. 4 connection is closing server: 7 FIN received by server, 6 FIN received by client, 6 FIN sent by client, 7 FIN sent by server Server has been closed: 7 FIN received by server, 6 FIN received by client, 6 FIN sent by client, 7 FIN sent by server ...

Copy link
Contributor

@cjihrigcjihrig left a comment

Choose a reason for hiding this comment

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

LGTM. And that CI failure is indeed unrelated.

@jasnell
Copy link
Member

@danbev ... just fyi... when including a large block of text like that, perhaps wrap it in a details tag... e.g.

For example
<details> <summary>Some stuff</summary>{markdown here} </details>

@danbev
Copy link
ContributorAuthor

just fyi... when including a large block of text like that, perhaps wrap it in a details tag... e.g.

Ah that is very useful. Thanks!

danbev added a commit to danbev/node that referenced this pull request May 15, 2017
I cannot find any usage of uv in the header and think that it can be removed. PR-URL: nodejs#12973 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
@danbev
Copy link
ContributorAuthor

Landed in 32f01c8

@danbevdanbev closed this May 15, 2017
@danbevdanbev deleted the asyncwrap-remove-unused-uv.h branch May 15, 2017 05:39
anchnk pushed a commit to anchnk/node that referenced this pull request May 19, 2017
I cannot find any usage of uv in the header and think that it can be removed. PR-URL: nodejs#12973 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
@jasnelljasnell mentioned this pull request May 28, 2017
@gibfahngibfahn mentioned this pull request Jun 15, 2017
3 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@danbev@jasnell@kurayama@thefourtheye@addaleax@cjihrig@MylesBorins@nodejs-github-bot