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

src: remove unused uv.h include in async-wrap.h #12973

Closed
wants to merge 1 commit into from

Conversation

@danbev
Copy link
Member

commented May 11, 2017

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

src: remove unused uv.h include in async-wrap.h
I cannot find any usage of uv in the header and think that it can be
removed.
@danbev

This comment has been minimized.

Copy link
Member Author

commented May 11, 2017

@thefourtheye
Copy link
Contributor

left a comment

LGTM if the CI is happy.

@danbev

This comment has been minimized.

Copy link
Member Author

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
  ...
@cjihrig
Copy link
Contributor

left a comment

LGTM. And that CI failure is indeed unrelated.

@jasnell

This comment has been minimized.

Copy link
Member

commented May 11, 2017

@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

This comment has been minimized.

Copy link
Member Author

commented May 12, 2017

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
src: remove unused uv.h include in async-wrap.h
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 <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danbev

This comment has been minimized.

Copy link
Member Author

commented May 15, 2017

Landed in 32f01c8

@danbev danbev closed this May 15, 2017

@danbev danbev deleted the danbev:asyncwrap-remove-unused-uv.h branch May 15, 2017

anchnk pushed a commit to anchnk/node that referenced this pull request May 19, 2017
src: remove unused uv.h include in async-wrap.h
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 <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell jasnell referenced this pull request May 28, 2017
@gibfahn gibfahn referenced this pull request Jun 15, 2017
2 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.