Skip to content

doc: improve child_process maxBuffer text #11791

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
Closed
wants to merge 1 commit into from
Closed

doc: improve child_process maxBuffer text #11791

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
addaleax
addaleax approved these changes Mar 10, 2017
View reviewed changes
doc/api/child_process.md Outdated
Copy link
Member

@addaleax addaleax Mar 10, 2017

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

@Trott Trott Mar 10, 2017

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 •
edited
Loading

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.

@Trott
doc: improve child_process maxBuffer text
5031022 
`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
@Trott
doc: improve child_process maxBuffer text
ea5a2f4 
`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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@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
@Trott
doc: improve child_process maxBuffer text
6cc7b30 
`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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@italoacasas italoacasas mentioned this pull request Mar 14, 2017
Merged
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
@Trott @jungx098
doc: improve child_process maxBuffer text
a624472 
`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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins
Copy link
Contributor

MylesBorins commented Apr 17, 2017

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".

@koh110 koh110 mentioned this pull request Oct 22, 2018
Closed
4 tasks
@Trott Trott deleted the maxbuffer-edit branch January 13, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees

No one assigned

Labels

child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Trott @MylesBorins @jasnell @addaleax @lpinca @cjihrig @nodejs-github-bot