FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: make Buffer documentation styles consistent by TimothyGu · Pull Request #4873 · nodejs/node · GitHub

/ node Public

doc: make Buffer documentation styles consistent - #4873

Closed
TimothyGu wants to merge 1 commit into
nodejs:masterfrom
TimothyGu:buffer-doc
ConversationCommits (1)ChecksFiles changed
Closed

doc: make Buffer documentation styles consistent#4873
TimothyGu wants to merge 1 commit into
nodejs:masterfrom
TimothyGu:buffer-doc

Conversation

TimothyGu commented Jan 26, 2016

Copy link
Copy Markdown
Member
  • Maintain alphabetical order
  • Add documentation for offset and value where absent
  • Add return value documentation where absent
  • Remove redundant "Optional"
  • Move defaults to parameter enumerations

TimothyGu changed the title doc: maintain alphabetical order of buffer.entries/keys/values doc: make Buffer documentation styles consistent Jan 26, 2016
mscdex added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels Jan 26, 2016
tflanagan reviewed Jan 26, 2016
View reviewed changes
Comment thread doc/api/buffer.markdown Outdated

tflanagan Jan 26, 2016

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

0 <= offset <= buf.length is very "techy", but I can't think of a better way to describe this requirement.

Offset of < 0 or > byteLength would throw new RangeError('Index out of range')

Maybe a paragraph about this error (not sure where) instead of a "techy" requirement expression?

TimothyGu Jan 26, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Well, I'd say that anybody reading documentation on a module designed to do binary stuff "techy" :P

It should be fairly obvious and intuitive that one cannot read (or write) out of range, so I'd argue against having a separate paragraph for such a simple constraint.

How about:

  • offset Number Must be between 0 and the length of the buffer, inclusive

I don't think it is any clearer than the pseudo-JavaScript expression however.

silverwind Jan 26, 2016

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Shouldn't it actually be 0 <= offset < buf.length? Anyways, I'd reduce the description to just "Zero-based byte offset" or similar.

jasnell commented Jan 27, 2016

Copy link
Copy Markdown
Member

@TimothyGu .. thank you for the PR! Can I ask you to please fill the PR text in with some detail about the change (just copying over the commit text would suffice). Also, it looks like the PR needs to be rebased and updated.

silverwind reviewed Jan 28, 2016
View reviewed changes
Comment thread doc/api/buffer.markdown Outdated

silverwind Jan 28, 2016

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Forgot to wrap?

Qard mentioned this pull request Jan 28, 2016
Closed

TimothyGu commented Jan 30, 2016

Copy link
Copy Markdown
Member Author

@jasnell, PR updated. All comments addressed as well.

jasnell commented Jan 30, 2016

Copy link
Copy Markdown
Member

Awesome! Thank you! LGTM
@nodejs/documentation

thefourtheye reviewed Jan 30, 2016
View reviewed changes
Comment thread doc/api/buffer.markdown

thefourtheye Jan 30, 2016

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Return value is missing

TimothyGu Jan 30, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed.

doc: make buffer methods styles consistent
4d550e8 
- Maintain alphabetical order
- Add documentation for `offset` and `value` where absent
- Add return value documentation where absent
- Remove redundant "Optional"
- Move defaults to parameter enumerations
Trott pushed a commit that referenced this pull request Jan 30, 2016
doc: make buffer methods styles consistent
6ad1f7b 
- Maintain alphabetical order
- Add documentation for `offset` and `value` where absent
- Add return value documentation where absent
- Remove redundant "Optional"
- Move defaults to parameter enumerations

PR-URL: #4873
Reviewed-By: James M Snell <jasnell@gmail.com>

Trott commented Jan 30, 2016

Copy link
Copy Markdown
Member

Landed in 6ad1f7b

Trott closed this Jan 30, 2016
TimothyGu deleted the buffer-doc branch January 30, 2016 19:24
rvagg pushed a commit that referenced this pull request Feb 8, 2016
doc: make buffer methods styles consistent
d47dadc 
- Maintain alphabetical order
- Add documentation for `offset` and `value` where absent
- Add return value documentation where absent
- Remove redundant "Optional"
- Move defaults to parameter enumerations

PR-URL: #4873
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg mentioned this pull request Feb 8, 2016
Closed

MylesBorins commented Feb 17, 2016

Copy link
Copy Markdown
Contributor

Blocked from backporting until other PR's are merged first

MylesBorins pushed a commit that referenced this pull request Feb 22, 2016
doc: make buffer methods styles consistent
eb56ace 
- Maintain alphabetical order
- Add documentation for `offset` and `value` where absent
- Add return value documentation where absent
- Remove redundant "Optional"
- Move defaults to parameter enumerations

PR-URL: #4873
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 22, 2016
doc: make buffer methods styles consistent
047f4a1 
- Maintain alphabetical order
- Add documentation for `offset` and `value` where absent
- Add return value documentation where absent
- Remove redundant "Optional"
- Move defaults to parameter enumerations

PR-URL: #4873
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
doc: make buffer methods styles consistent
aeac623 
- Maintain alphabetical order
- Add documentation for `offset` and `value` where absent
- Add return value documentation where absent
- Remove redundant "Optional"
- Move defaults to parameter enumerations

PR-URL: #4873
Reviewed-By: James M Snell <jasnell@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
doc: make buffer methods styles consistent
c70dcda 
- Maintain alphabetical order
- Add documentation for `offset` and `value` where absent
- Add return value documentation where absent
- Remove redundant "Optional"
- Move defaults to parameter enumerations

PR-URL: nodejs#4873
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced Jul 7, 2023
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Merged
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
snyk-io Bot mentioned this pull request Oct 21, 2024
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

buffer Issues and PRs related to the buffer 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.

9 participants

Back | FazBrowse Home | New Git URL