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: fix unit of size argument of readable.read #35051

Closed

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented Sep 4, 2020

In the context of data transmission, "GB" refers to 109 bytes, whereas the actual limit is 230. The correct unit symbol is "GiB".

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

"GB" refers to 10**9 bytes, whereas the actual limit is 2**30. The
correct unit symbol is "GiB".
@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 Sep 4, 2020
@lpinca
Copy link
Member

lpinca commented Sep 4, 2020

I guess this depends on the base used. I think both are acceptable.

@tniessen
Copy link
Member Author

tniessen commented Sep 4, 2020

I guess this depends on the base used. I think both are acceptable.

@lpinca The point is that, even if it isn't incorrect, it is at least ambiguous.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@tniessen tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 6, 2020
@@ -1126,7 +1126,7 @@ buffer will be returned.
If the `size` argument is not specified, all of the data contained in the
internal buffer will be returned.

The `size` argument must be less than or equal to 1 GB.
The `size` argument must be less than or equal to 1 GiB.
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker, as if anyone has any great ideas, they can always be implemented later, but I think the distinction is esoteric enough that we should spell it out on the first occurrence (or maybe even every occurrence). But I'm also having a hard time figuring out a way to do it in a straightforward way. 1024^3 bytes is not exactly immediately clarifying. (My thought would be, "Isn't that the same as a GB?")

@tniessen tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 8, 2020
@tniessen tniessen force-pushed the doc-be-explicit-about-gib-unit branch from 7861793 to 277e12c Compare September 8, 2020 12:24
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 8, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2020

Commit Queue failed
- Loading data for nodejs/node/pull/35051
✔  Done loading data for nodejs/node/pull/35051
----------------------------------- PR info ------------------------------------
Title      doc: fix unit of size argument of readable.read (#35051)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     tniessen:doc-be-explicit-about-gib-unit -> nodejs:master
Labels     author ready, doc, stream
Commits    1
 - doc: fix unit of size argument of readable.read
Committers 1
 - Tobias Nießen 
PR-URL: https://github.com/nodejs/node/pull/35051
Reviewed-By: Luigi Pinca 
Reviewed-By: Matteo Collina 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/35051
Reviewed-By: Luigi Pinca 
Reviewed-By: Matteo Collina 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - doc: fix unit of size argument of readable.read
   ✖  GitHub CI is still running
   ℹ  Doc-only changes
   ℹ  This PR was created on Fri, 04 Sep 2020 09:55:48 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/35051#pullrequestreview-482573171
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/35051#pullrequestreview-482739095
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

@github-actions github-actions bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 8, 2020
@tniessen tniessen removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 16, 2020
@tniessen tniessen force-pushed the doc-be-explicit-about-gib-unit branch from 277e12c to 7861793 Compare September 16, 2020 14:59
@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 11, 2020
@Flarna Flarna added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 13, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 13, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Flarna Flarna added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 13, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 13, 2020
@github-actions
Copy link
Contributor

Landed in 2cfdf28...d7e5d65

@github-actions github-actions bot closed this Oct 13, 2020
nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2020
"GB" refers to 10**9 bytes, whereas the actual limit is 2**30. The
correct unit symbol is "GiB".

PR-URL: #35051
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
MylesBorins pushed a commit that referenced this pull request Oct 14, 2020
"GB" refers to 10**9 bytes, whereas the actual limit is 2**30. The
correct unit symbol is "GiB".

PR-URL: #35051
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@MylesBorins MylesBorins mentioned this pull request Oct 14, 2020
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
"GB" refers to 10**9 bytes, whereas the actual limit is 2**30. The
correct unit symbol is "GiB".

PR-URL: nodejs#35051
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

8 participants