FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

docs: improve process event headers by BridgeAR · Pull Request #20312 · nodejs/node · GitHub

/ node Public

docs: improve process event headers - #20312

Closed
BridgeAR wants to merge 1 commit into
nodejs:masterfrom
BridgeAR:improve-process-docs
ConversationCommits (1)ChecksFiles changed
Closed

docs: improve process event headers#20312
BridgeAR wants to merge 1 commit into
nodejs:masterfrom
BridgeAR:improve-process-docs

Conversation

BridgeAR commented Apr 26, 2018

Copy link
Copy Markdown
Member

The headers should be handled as all others as well and just indicate
all arguments.

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

docs: improve process event headers
07d674c 
The headers should be handled as all others as well and just indicate
all arguments.
BridgeAR requested a review from vsemozhetbyt April 26, 2018 00:20
nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 26, 2018

BridgeAR commented Apr 26, 2018

Copy link
Copy Markdown
Member Author

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

vsemozhetbyt added the process Issues and PRs related to the process subsystem. label Apr 26, 2018
vsemozhetbyt approved these changes Apr 26, 2018
View reviewed changes

vsemozhetbyt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

With a nit concern)

Comment thread doc/api/process.md
added: v0.5.10
-->

* `message` {Object} a parsed JSON object or primitive value.

vsemozhetbyt Apr 26, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I am not sure if we should explicitly enumerate "primitive values" here. What types can they be?
{Object | boolean | number | string | null }?
It seems any does not suffice as there hardly can be undefined or symbol.

BridgeAR Apr 26, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hm, I have no strong opinion. @Trott maybe?

BridgeAR Apr 27, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Since I did not change the current state, I guess this can be addressed at a different point of time as well?

vsemozhetbyt Apr 29, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Addressed)

BridgeAR commented Apr 27, 2018

Copy link
Copy Markdown
Member Author

@nodejs/documentation PTAL

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Apr 27, 2018
BridgeAR added a commit to BridgeAR/node that referenced this pull request Apr 29, 2018
doc: improve process event headers
b92c656 
The headers should be handled as all others as well and just indicate
all arguments.

PR-URL: nodejs#20312
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

BridgeAR commented Apr 29, 2018

Copy link
Copy Markdown
Member Author

Landed in b92c656

BridgeAR closed this Apr 29, 2018
MylesBorins pushed a commit that referenced this pull request May 4, 2018
doc: improve process event headers
5542a98 
The headers should be handled as all others as well and just indicate
all arguments.

PR-URL: #20312
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins mentioned this pull request May 8, 2018
Merged
BridgeAR deleted the improve-process-docs branch January 20, 2020 11:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
trivikr trivikr approved these changes
+1 more reviewer
vsemozhetbyt vsemozhetbyt approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL