Navigation Menu

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: The default value of highWaterMark is 16KB. #31561

Closed
wants to merge 2 commits into from

Conversation

yunnysunny
Copy link
Contributor

The default value of highWaterMark is 16KB, but it is 16kb in the stream API document. I think it is just a spelling mistake.

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

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Jan 29, 2020
Trott
Trott previously requested changes Jan 31, 2020
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Hi, @yunnysunny! Welcome, and thank you for the pull request. Can you also fix the occurrence of kb on line 1429? That will at least keep this file consistent with itself. Unfortunately, we seem to use kb, KB, and K throughout the documents.

Change `64kb` to `64KB` in  `stream.md`
@yunnysunny
Copy link
Contributor Author

@Trott I have corrected it.

@Trott Trott dismissed their stale review February 1, 2020 22:52

consistency achieved

@ronag ronag requested a review from Trott April 30, 2020 09:08
@ronag ronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 30, 2020
@nodejs-github-bot
Copy link
Collaborator

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

Copy link
Member

@juanarbol juanarbol left a comment

Choose a reason for hiding this comment

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

LGTM

juanarbol pushed a commit that referenced this pull request Apr 30, 2020
Change `64kb` to `64KB` in  `stream.md`

PR-URL: #31561
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
@juanarbol
Copy link
Member

Thanks for your contribution, landed in e951825 🎉

@juanarbol juanarbol closed this Apr 30, 2020
targos pushed a commit that referenced this pull request May 4, 2020
Change `64kb` to `64KB` in  `stream.md`

PR-URL: #31561
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
@targos targos mentioned this pull request May 4, 2020
targos pushed a commit that referenced this pull request May 7, 2020
Change `64kb` to `64KB` in  `stream.md`

PR-URL: #31561
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
targos pushed a commit that referenced this pull request May 13, 2020
Change `64kb` to `64KB` in  `stream.md`

PR-URL: #31561
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants