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.

Sign up for GitHub

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

Jump to bottom

doc: add process api data types to documentation #9505

Closed
wants to merge 4 commits into from
Closed

doc: add process api data types to documentation #9505

wants to merge 4 commits into from

Conversation

imatveev
Copy link
Contributor

@imatveev imatveev commented Nov 7, 2016 •
edited
Loading

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 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 imatveev mentioned this pull request Nov 7, 2016
Closed
mscdex
mscdex reviewed Nov 8, 2016
View reviewed changes
doc/api/process.md
@@ -852,13 +871,15 @@ And `process.argv`:
added: v0.1.100
-->

* {String}

The `process.execPath` property returns the absolute pathname of the executable
that started the Node.js process.

For example:

```sh
Copy link
Contributor

Choose a reason for hiding this comment

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

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

mscdex
mscdex reviewed Nov 8, 2016
View reviewed changes
doc/api/process.md
@@ -603,6 +613,9 @@ added: v6.1.0

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

@mscdex mscdex Nov 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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

mscdex
mscdex reviewed Nov 8, 2016
View reviewed changes
doc/api/process.md
@@ -931,6 +952,8 @@ is safer than calling `process.exit()`.
added: v0.11.8
-->

* {Number}
Copy link
Contributor

Choose a reason for hiding this comment

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

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

mscdex
mscdex reviewed Nov 8, 2016
View reviewed changes
doc/api/process.md
@@ -1008,6 +1037,8 @@ Android)
added: v0.1.28
-->

* return: {Number}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto about using {Integer}.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

@imatveev
Copy link
Contributor Author

imatveev commented Nov 8, 2016

Added changes according to @mscdex review.

lpinca
lpinca reviewed Nov 8, 2016
View reviewed changes
doc/api/process.md
@@ -1634,6 +1685,8 @@ console.log(`Version: ${process.version}`);
added: v0.2.0
-->

* Return: {Object}
Copy link
Member

Choose a reason for hiding this comment

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

I think "Return:" should be removed here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed.

@mscdex
Copy link
Contributor

mscdex commented Nov 8, 2016

LGTM

cjihrig
cjihrig reviewed Nov 8, 2016
View reviewed changes
doc/api/process.md
@@ -1576,6 +1622,7 @@ See the [TTY][] documentation for more information.
added: v0.1.104
-->

* {String}
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

mhdawson
mhdawson approved these changes Nov 8, 2016
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@silverwind silverwind mentioned this pull request Nov 11, 2016
Closed
2 tasks
@silverwind
Copy link
Contributor

Thanks! Landed in c184f22.

@silverwind silverwind closed this Nov 11, 2016
silverwind pushed a commit that referenced this pull request Nov 11, 2016
@imatveev @silverwind
doc: add process api data types to documentation
c184f22 
PR-URL: #9505
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
addaleax pushed a commit that referenced this pull request Nov 22, 2016
@imatveev @addaleax
doc: add process api data types to documentation
77aded3 
PR-URL: #9505
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@MylesBorins
Copy link
Contributor

is this accurate for v4 or v6?

@imatveev
Copy link
Contributor Author

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

@imatveev 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
@imatveev @MylesBorins
doc: add process api data types to documentation
d2a1a67 
PR-URL: #9505
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@imatveev @MylesBorins
doc: add process api data types to documentation
913f1bd 
PR-URL: #9505
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 21, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mscdex mscdex mscdex left review comments

@mhdawson mhdawson mhdawson approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@silverwind silverwind silverwind approved these changes

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
@imatveev @mscdex @silverwind @MylesBorins @lpinca @cjihrig @mhdawson @nodejs-github-bot