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: improve child_process maxBuffer text #11791

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Mar 10, 2017

maxBuffer information in child_process.md used atypical formatting.
This uses a single consistent style for all instances of maxBuffer
information.

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

doc child_process

@Trott Trott added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Mar 10, 2017
@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Mar 10, 2017
* [`maxBuffer`][] {number} largest amount of data (in bytes) allowed on
stdout or stderr - if exceeded child process is killed (Default: `200*1024`)
* `maxBuffer` {number} Largest amount of data in bytes allowed on stdout or
stderr. (Default: `200*1024`) If exceede, the child process is terminated.
Copy link
Member

Choose a reason for hiding this comment

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

typo: exceede

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, thanks!

@Trott
Copy link
Member Author

Trott commented Mar 10, 2017

A key thing this fixes is the confusing alternate formatting for maxBuffer in the doc which you can see here:


screen shot 2017-03-10 at 9 47 57 am


Because it's a link, maxBuffer is formatted differently than shell, timeout, uid, gid...

The link isn't to full information about maxBuffer so it's a bit misleading. It's a special note about maxBuffer and Unicode. So this PR moves the link out of that list and into the descriptive text, and makes it clear that it's a note and not key information. The note basically reminds people that a character can be more than one byte.

`maxBuffer` information in child_process.md used atypical formatting.
This uses a single consistent style for all instances of `maxBuffer`
information.
Trott added a commit to Trott/io.js that referenced this pull request Mar 13, 2017
`maxBuffer` information in child_process.md used atypical formatting.
This uses a single consistent style for all instances of `maxBuffer`
information.

PR-URL: nodejs#11791
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member Author

Trott commented Mar 13, 2017

Landed in ea5a2f4

@Trott Trott closed this Mar 13, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Mar 14, 2017
`maxBuffer` information in child_process.md used atypical formatting.
This uses a single consistent style for all instances of `maxBuffer`
information.

PR-URL: nodejs#11791
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
`maxBuffer` information in child_process.md used atypical formatting.
This uses a single consistent style for all instances of `maxBuffer`
information.

PR-URL: nodejs#11791
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

is this applicable to v6.x?

@Trott
Copy link
Member Author

Trott commented Apr 18, 2017

@MylesBorins It's a style change, not a content change, so I think the answer is "yes, but it's also not crucial by any stretch".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants