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

doc: add process api data types to documentation by imatveev · Pull Request #9505 · nodejs/node · GitHub

/ node Public

doc: add process api data types to documentation - #9505

Closed
imatveev wants to merge 4 commits into
nodejs:masterfrom
imatveev:docs-process-props-data-type
ConversationCommits (4)ChecksFiles changed
Closed

doc: add process api data types to documentation#9505
imatveev wants to merge 4 commits into
nodejs:masterfrom
imatveev:docs-process-props-data-type

Conversation

imatveev commented Nov 7, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor
Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Clarified part of data types of properties and return values of process module in docs due to item

nodejs-github-bot added doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem. labels Nov 7, 2016
imatveev mentioned this pull request Nov 7, 2016
Closed
mscdex reviewed Nov 8, 2016
View reviewed changes
Comment thread doc/api/process.md Outdated

For example:

```sh

mscdex Nov 8, 2016

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

This shouldn't be sh if we're converting it to a JavaScript string value, it should be js.

mscdex reviewed Nov 8, 2016
View reviewed changes
Comment thread doc/api/process.md Outdated

* `previousValue` {Object} A previous return value from calling
`process.cpuUsage()`
* return: {Object}

mscdex Nov 8, 2016 •
edited
Loading

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

Can you capitalize every instance of 'return' to look like 'Return'?

mscdex reviewed Nov 8, 2016
View reviewed changes
Comment thread doc/api/process.md Outdated
added: v0.11.8
-->

* {Number}

mscdex Nov 8, 2016

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

You might be more specific here and use {Integer} instead.

mscdex reviewed Nov 8, 2016
View reviewed changes
Comment thread doc/api/process.md Outdated
added: v0.1.28
-->

* return: {Number}

mscdex Nov 8, 2016

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

Ditto about using {Integer}.

mscdex Nov 8, 2016

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

Same goes for any other non-floating point number values.

imatveev commented Nov 8, 2016

Copy link
Copy Markdown
Contributor Author

Added changes according to @mscdex review.

lpinca reviewed Nov 8, 2016
View reviewed changes
Comment thread doc/api/process.md Outdated
added: v0.2.0
-->

* Return: {Object}

lpinca Nov 8, 2016

Copy link
Copy Markdown
Member

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 think "Return:" should be removed here.

mscdex Nov 8, 2016

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

Agreed.

mscdex commented Nov 8, 2016

Copy link
Copy Markdown
Contributor

LGTM

cjihrig reviewed Nov 8, 2016
View reviewed changes
Comment thread doc/api/process.md
added: v0.1.104
-->

* {String}

cjihrig Nov 8, 2016

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

For consistency with the other changes, maybe add a blank line after this line.

imatveev Nov 8, 2016

Copy link
Copy Markdown
Contributor 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

done

mhdawson approved these changes Nov 8, 2016
View reviewed changes

mhdawson left a comment

Copy link
Copy Markdown
Member

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

LGTM

silverwind mentioned this pull request Nov 11, 2016
Closed
2 tasks

silverwind commented Nov 11, 2016

Copy link
Copy Markdown
Contributor

Thanks! Landed in c184f22.

silverwind closed this Nov 11, 2016
silverwind pushed a commit that referenced this pull request Nov 11, 2016
doc: add process api data types to documentation
c184f22 
PR-URL: #9505
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
addaleax pushed a commit that referenced this pull request Nov 22, 2016
doc: add process api data types to documentation
77aded3 
PR-URL: #9505
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>

MylesBorins commented Dec 13, 2016

Copy link
Copy Markdown
Contributor

is this accurate for v4 or v6?

imatveev commented Dec 14, 2016

Copy link
Copy Markdown
Contributor Author

Checked on V6, can check on V4 if it needs to be done.

imatveev deleted the docs-process-props-data-type branch December 15, 2016 22:26
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
doc: add process api data types to documentation
d2a1a67 
PR-URL: #9505
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
doc: add process api data types to documentation
913f1bd 
PR-URL: #9505
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
MylesBorins mentioned this pull request Dec 21, 2016
Merged
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

lpinca lpinca approved these changes
cjihrig cjihrig approved these changes
+3 more reviewers
mscdex mscdex left review comments
silverwind silverwind approved these changes
mhdawson mhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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.

8 participants

Back | FazBrowse Home | New Git URL