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

doc: Add notes how addMembership should be managed #16240

Closed
wants to merge 1 commit into from

Conversation

watilde
Copy link
Member

@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
Member 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
Member 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
Copy link
Member 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 <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Oct 24, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 26, 2018
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
BethGriggs pushed a commit that referenced this pull request Mar 7, 2019
Fixes: #12572
Refs: #16240

PR-URL: #23746
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
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