Skip to content

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 13, 2017

I cannot find any usages of these includes and think they can be
removed.

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

src

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 13, 2017
@danbev
Copy link
Contributor Author

danbev commented Oct 13, 2017

@mscdex
Copy link
Contributor

mscdex commented Oct 13, 2017

Minor nit: 'includeds' should probably be 'includes' in the commit message

I cannot find any usages of these includes and think they can be
removed.
@danbev
Copy link
Contributor Author

danbev commented Oct 13, 2017

Minor nit: 'includeds' should probably be 'includes' in the commit message

Indeed, I'll fix that. Thanks!

@danbev danbev force-pushed the unused-includes-node_wrap.h branch from f136a1d to 02f8cda Compare October 13, 2017 15:49
@danbev danbev changed the title src: remove unused includeds from node_wrap.h src: remove unused includes from node_wrap.h Oct 13, 2017
@Trott
Copy link
Member

Trott commented Oct 13, 2017

CI did not run any subtasks. (There's a problem that @nodejs/build may or may not have fixed by now.) It will need to be re-run.

@joyeecheung
Copy link
Member

jasnell pushed a commit that referenced this pull request Oct 15, 2017
I cannot find any usages of these includes and think they can be
removed.

PR-URL: #16179
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Oct 15, 2017

Landed in 75f154b

@jasnell jasnell closed this Oct 15, 2017
targos pushed a commit that referenced this pull request Oct 18, 2017
I cannot find any usages of these includes and think they can be
removed.

PR-URL: #16179
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 18, 2017
I cannot find any usages of these includes and think they can be
removed.

PR-URL: nodejs/node#16179
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danbev danbev deleted the unused-includes-node_wrap.h branch November 16, 2017 08:36
@MylesBorins
Copy link
Contributor

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

danbev added a commit to danbev/node that referenced this pull request Nov 17, 2017
I cannot find any usages of these includes and think they can be
removed.

PR-URL: nodejs#16179
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
I cannot find any usages of these includes and think they can be
removed.

Backport-PR-URL: #17092
PR-URL: #16179
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
I cannot find any usages of these includes and think they can be
removed.

Backport-PR-URL: #17092
PR-URL: #16179
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 28, 2017
Sign up for free to 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.

9 participants