Skip to content

Conversation

watilde
Copy link
Contributor

@watilde watilde commented Oct 16, 2017

Fixes: #12572

Checklist
Affected core subsystem(s)

doc, dgram

@nodejs-github-bot nodejs-github-bot added dgram Issues and PRs related to the dgram subsystem / UDP. doc Issues and PRs related to the documentations. labels Oct 16, 2017
doc/api/dgram.md Outdated
@@ -95,6 +95,17 @@ Tells the kernel to join a multicast group at the given `multicastAddress` and
one interface and will add membership to it. To add membership to every
available interface, call `addMembership` multiple times, once per interface.

When the socket is created, each worker gets a copy and they refer the
Copy link
Member

Choose a reason for hiding this comment

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

Does "worker" refer to a cluster environment? If so I think it makes sense to specify it as the sentence seems to be a bit out of context without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it does and I agreed on it. I will add the context.

doc/api/dgram.md Outdated
@@ -95,6 +95,17 @@ Tells the kernel to join a multicast group at the given `multicastAddress` and
one interface and will add membership to it. To add membership to every
available interface, call `addMembership` multiple times, once per interface.

When the socket is created, each worker gets a copy and they refer the
same socket so you need to manage to call `.addMembership` only once.
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid using the informal pronoun you in the docs :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I will update it.

@watilde watilde force-pushed the feature/docs-dgram branch from 1c7f640 to 5292a50 Compare October 22, 2017 09:36
@watilde
Copy link
Contributor Author

watilde commented Oct 25, 2017

@watilde watilde closed this Nov 14, 2017
@watilde watilde deleted the feature/docs-dgram branch November 14, 2017 21:37
@prog1dev
Copy link
Contributor

prog1dev commented Feb 2, 2018

@watilde Why did you close this pr?

@gireeshpunathil
Copy link
Member

ping @watilde - what happened here? the changes look reasonable. was it through an error you deleted the branch and closed the PR? are you planning to resurrect this?

jasnell added a commit to jasnell/node that referenced this pull request Oct 19, 2018
jasnell added a commit that referenced this pull request Oct 23, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request Oct 24, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 26, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
BethGriggs pushed a commit that referenced this pull request Mar 7, 2019
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants