★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/18330Nouvelle recherche | Portail wanayoo
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

process: small improvements to internals #18330

Closed

Conversation

Copy link
Member

@apapirovski apapirovski commented Jan 23, 2018

Some small tweaks:

  1. Remove unnecessary call to a binding which is already guaranteed to be loaded via requiring child_process (and that's even if calling that binding was necessary in the first place)

  2. Cleanup setupSignalHandlers, including removing an outdated comment, using null prototype Object, not calling C++ repeatedly to get the binding, etc.

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

process

@nodejs-github-bot nodejs-github-bot added the process label Jan 23, 2018
@apapirovski
Copy link
Member Author

@apapirovski apapirovski commented Jan 23, 2018

@maclover7 maclover7 added the author ready label Jan 26, 2018
@maclover7
Copy link
Member

@maclover7 maclover7 commented Jan 26, 2018

Landing...

@maclover7 maclover7 self-assigned this Jan 26, 2018
@maclover7
Copy link
Member

@maclover7 maclover7 commented Jan 26, 2018

Landed in a5a8118, 09ef021 (kept separate in very unlikely chance needs to be reverted)

@maclover7 maclover7 closed this Jan 26, 2018
maclover7 pushed a commit to maclover7/node that referenced this issue Jan 26, 2018
PR-URL: nodejs#18330
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
maclover7 pushed a commit to maclover7/node that referenced this issue Jan 26, 2018
PR-URL: nodejs#18330
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
evanlucas pushed a commit that referenced this issue Jan 30, 2018
PR-URL: #18330
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
evanlucas pushed a commit that referenced this issue Jan 30, 2018
PR-URL: #18330
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
@addaleax addaleax removed the author ready label Feb 4, 2018
@MylesBorins MylesBorins added lts-watch-v6.x baking-for-lts labels Feb 27, 2018
@MylesBorins
Copy link
Member

@MylesBorins MylesBorins commented Feb 27, 2018 •

this might need a bit of time for lts

MayaLekova pushed a commit to MayaLekova/node that referenced this issue May 8, 2018
PR-URL: nodejs#18330
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MayaLekova pushed a commit to MayaLekova/node that referenced this issue May 8, 2018
PR-URL: nodejs#18330
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
@BethGriggs BethGriggs removed baking-for-lts lts-watch-v8.x labels Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants