Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

docs: rework Buffer page#1388

Merged
benhalverson merged 5 commits into
nodejs:masterfrom
addaleax:rework-buffers
Jun 8, 2021
Merged

docs: rework Buffer page#1388
benhalverson merged 5 commits into
nodejs:masterfrom
addaleax:rework-buffers

Conversation

@addaleax

@addaleax addaleax commented Jun 6, 2021

Copy link
Copy Markdown
Member

Description

  • Remove wildly misleading text about how Buffers in Node.js are
    related to the concept of buffering (they are not!)
  • Talk about UTF-8 instead of Unicode when UTF-8 is what the text
    is referring to and mention that it is dangerous to act as if
    bytes in a Buffer and characters are in 1:1 correspondence (they are not!)
  • Recommend using standardized JS methods instead of
    Node.js-specific ones where possible

Related Issues

Refs: #1387 (review)

- Remove wildly misleading text about how Buffers in Node.js are
  related to the concept of buffering
- Talk about UTF-8 instead of Unicode when UTF-8 is what the text
  is referring to and mention that it is dangerous to act as if
  bytes in a Buffer and characters are in 1:1 correspondence
- Recommend using standardized JS methods instead of
  Node.js-specific ones where possible

Refs: nodejs#1387 (review)
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #1388 (7550336) into master (bd7848c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1388   +/-   ##
=======================================
  Coverage   85.24%   85.24%           
=======================================
  Files          64       64           
  Lines         732      732           
  Branches      214      214           
=======================================
  Hits          624      624           
  Misses        106      106           
  Partials        2        2           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd7848c...7550336. Read the comment docs.

@bl-ue

bl-ue commented Jun 6, 2021

Copy link
Copy Markdown
Contributor

You should add your name to authors: at the top 😉

@addaleax

addaleax commented Jun 6, 2021

Copy link
Copy Markdown
Member Author

@bl-ue Thank you, done :)

@bl-ue bl-ue left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few suggestions — all optional, of course :)

Comment thread src/documentation/0051-node-buffers/index.md Outdated
Comment thread src/documentation/0051-node-buffers/index.md Outdated
addaleax and others added 2 commits June 7, 2021 00:10
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
@bl-ue

bl-ue commented Jun 6, 2021

Copy link
Copy Markdown
Contributor

You know, these docs look like a rich opportunity for grammatical expertise — I'll improve them in my spare time 🤔

Comment thread src/documentation/0051-node-buffers/index.md Outdated
Co-authored-by: Rich Trott <rtrott@gmail.com>
@benhalverson benhalverson merged commit 5f9c56d into nodejs:master Jun 8, 2021
@addaleax addaleax deleted the rework-buffers branch June 8, 2021 17:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants