doc: add options description for process.send() and worker.send() by dev-script · Pull Request #29868 · nodejs/node · GitHub

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

This is my first PR, Please review. Thank you

This is missing from the signature above

@mscdex thanks, i fix it.

@mscdex please can you give me some more idea about this.

@dev-313 - you need to modify the API signature at line 466 to add the options argument as well - like how did it for the worker.send API.

@gireeshpunathil please review now.

looks good. @mscdex - PTAL!

@BridgeAR should I close this PR.

@dev-313 if I am not mistaken this is an documentation fix. As such, it would be great to keep it open. Seems like there's just one comment left that should be addressed (worker.send(message[, sendHandle][, callback]) misses the options argument completely and that should be added there).
I just do not know what options are valid and which are not for this API; @addaleax maybe?

Suggested change

      
            * `options` {Object} The `options` argument, if present, is an object used to
          
            Object used to

so should remove the existing options {Object} line and add only Object used to , Am i right @Trott

Whoops, my mistake. Let me try again.

Suggested change

      
            * `options` {Object} The `options` argument, if present, is an object used to
          
            * `options` {Object} Used to

ok i understand @Trott , Thnaks

@Trott i have one more doubt all three files have to change (child_process, cluster, process) or only process.md

Only process.md.

Ok Thanks

@dev-313 I see you responded to a comment seven days ago, but it hasn't been addressed. Are you still working on this?

yes @Trott i'm working on this.

@Trott help why these stage 2 tests are failed.

Looks like you have a merge conflict to resolve.

Thanks @Trott , I'll fix it,instead of this everything is ok or not.

CI: https://ci.nodejs.org/job/node-test-pull-request/26073/

@Trott everytime i get this commit message error, please guide me how i can resolve this issue.
I know how to write a proper commit but confusion is that how to change this first commit message.

@Trott everytime i get this commit message error, please guide me how i can resolve this issue.
I know how to write a proper commit but confusion is that how to change this first commit message.

You need to rebase and amend the first commit message. That can happen when the pull request is landed so if you don't get to it, that's OK. You do need to make changes to address @mscdex's comment, though.

Describes the meaning of the boolean return in process.send() (doc/api/process.md) and worker.send() (doc/api/cluster.md) as described in subprocess.send() (doc/api/child_process.md) Fixes: #26995

Fixes: #26995

Add 'options' argument in worker.send() (doc/api/cluster.md) API signature. Fixes: #26995

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/4004/

all looks good to me. One more approval, and we should be ready to go!

invoking 7 day rule for PR with a single approval!

Describes the meaning of the boolean return in process.send() (doc/api/process.md) and worker.send() (doc/api/cluster.md) as described in subprocess.send() (doc/api/child_process.md) Fixes: #26995 PR-URL: #29868 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

landed as 5d1578d

thanks @dev-313 for your contribution!

Describes the meaning of the boolean return in process.send() (doc/api/process.md) and worker.send() (doc/api/cluster.md) as described in subprocess.send() (doc/api/child_process.md) Fixes: #26995 PR-URL: #29868 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 7, 2019

mscdex reviewed Oct 7, 2019

View reviewed changes

Comment thread package-lock.json Outdated

BridgeAR changed the title Added the description for the boolean return value of 'send()' in process.send() and worker.send() api. doc: add options description for process.send() and worker.send() Oct 9, 2019

dev-script closed this Oct 10, 2019

dev-script reopened this Oct 10, 2019

dev-script closed this Oct 10, 2019

BridgeAR reopened this Oct 10, 2019

Trott reviewed Oct 14, 2019

View reviewed changes

Trott reviewed Oct 16, 2019

View reviewed changes

gireeshpunathil approved these changes Oct 17, 2019

View reviewed changes

dev-script added 5 commits October 18, 2019 15:27

doc: add options description for process.send() and worker.send()

d6d21d6

Describes the meaning of the boolean return in process.send() (doc/api/process.md) and worker.send() (doc/api/cluster.md) as described in subprocess.send() (doc/api/child_process.md) Fixes: #26995

doc: change options description for process.send()

356dcb9

doc: change options description for process.md()

ba5bf9d

Fixes: #26995

doc: trim trailing whitespace from doc/api/process.md

691ebd2

doc: Changes in worker.send() API signature.

0ccba0d

Add 'options' argument in worker.send() (doc/api/cluster.md) API signature. Fixes: #26995

dev-script requested a review from Trott October 22, 2019 09:45

gireeshpunathil added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Oct 23, 2019

gireeshpunathil closed this Oct 30, 2019

targos mentioned this pull request Nov 5, 2019

v13.1.0 release proposal #30262

Merged

targos mentioned this pull request Nov 10, 2019

v12.13.1 release proposal #30352

Merged

	* `options` {Object} The `options` argument, if present, is an object used to
	Object used to

	* `options` {Object} The `options` argument, if present, is an object used to
	* `options` {Object} Used to

doc: add options description for process.send() and worker.send() by dev-script · Pull Request #29868 · nodejs/node · GitHub

Uh oh!

Uh oh!

doc: add options description for process.send() and worker.send() - #29868

doc: add options description for process.send() and worker.send()#29868dev-script wants to merge 5 commits intonodejs:masternodejs/node:masterfrom dev-script:add-devendra-shekhawatCopy head branch name to clipboard

Conversation

dev-script commented Oct 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

dev-script commented Oct 7, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dev-script commented Oct 10, 2019

Uh oh!

BridgeAR commented Oct 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trott commented Oct 14, 2019

Uh oh!

dev-script commented Oct 15, 2019

Uh oh!

dev-script commented Oct 16, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Oct 17, 2019

Uh oh!

dev-script commented Oct 17, 2019

Uh oh!

Trott commented Oct 18, 2019

Uh oh!

Trott commented Oct 22, 2019

Uh oh!

gireeshpunathil commented Oct 23, 2019

Uh oh!

gireeshpunathil commented Oct 30, 2019

Uh oh!

gireeshpunathil commented Oct 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

doc: add options description for process.send() and worker.send()#29868
dev-script wants to merge 5 commits into
nodejs:masterfrom
dev-script:add-devendra-shekhawat

dev-script commented Oct 7, 2019 •
edited
Loading