Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Update child_process.markdown, fix VIP example #25572

Closed
wants to merge 1 commit into from

Conversation

jalexanderfox
Copy link

The original logic would have sent the socket to both child processes instead of just one. By adding the else statement, it should now only send to one child process.

The original logic would have sent the socket to both child processes instead of just one. By adding the else statement, it should now only send to one child process.
@@ -308,9 +308,10 @@ socket to a "special" child process. Other sockets will go to a "normal" process
if (socket.remoteAddress === '74.125.127.100') {
special.send('socket', socket);
return;
}
} else {
// just the usual dudes
Copy link

Choose a reason for hiding this comment

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

Can you remove this line. I'm amazed the word dudes is here actually.

@jasnell
Copy link
Member

jasnell commented Aug 4, 2015

The addition of the else is unnecessary. The return statement within the if takes care of the potential control flow issue. That said, @cjihrig is right about the "dudes" comment. That should go.

@jasnell jasnell added the doc label Aug 14, 2015
@jasnell
Copy link
Member

jasnell commented Aug 14, 2015

Closing this here. I've opened an PR with a commit that removes the dudes from the example

@jasnell jasnell closed this Aug 14, 2015
jasnell added a commit to jasnell/node that referenced this pull request Aug 26, 2015
jasnell added a commit to nodejs/node that referenced this pull request Aug 26, 2015
per: nodejs/node-v0.x-archive#25572 (comment)

Reviewed By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: #2378
jasnell added a commit to nodejs/node that referenced this pull request Aug 26, 2015
per: nodejs/node-v0.x-archive#25572 (comment)

Reviewed By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: #2378
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants