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.

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

doc: add documentation for blob.bytes() method #54114

Closed
wants to merge 5 commits into from
Closed

doc: add documentation for blob.bytes() method #54114

wants to merge 5 commits into from

Conversation

jmsb02
Copy link
Contributor

@jmsb02 jmsb02 commented Jul 30, 2024 •
edited by jakecastelli
Loading

Description:
This pull request adds documentation for the blob.bytes() method in the buffer module. The method was introduced in Node.js v20.16.0, but it was not documented. This PR aims to provide a clear description and examples for the blob.bytes() method.

Motivation:
The blob.bytes() method allows users to obtain the bytes of a Blob object as a Buffer.
This method was introduced in a recent release but is currently undocumented.

Fixes: #54105

Documentation:

blob.bytes()

The blob.bytes() method returns the bytes of the Blob object as a Buffer.

const blob = new Blob(['hello']);
console.log(blob.bytes()); // Outputs: <Buffer 68 65 6c 6c 6f>

Additional Notes:
I included the text.txt file incorrectly while testing git push for the first time. I'd appreciate it if you could ignore it. I'll be more careful in the future!

@jmsb02 jmsb02 changed the title Add documentation for blob.bytes() method Add documentation for blob.bytes() method #54105 Jul 30, 2024
@jmsb02 jmsb02 changed the title Add documentation for blob.bytes() method #54105 Issue #54105: Add documentation for blob.bytes() method Jul 30, 2024
pluris
pluris reviewed Jul 30, 2024
View reviewed changes
test.txt Outdated Show resolved Hide resolved
pluris
pluris reviewed Jul 30, 2024
View reviewed changes
doc/api/buffer.md Outdated Show resolved Hide resolved
jakecastelli
jakecastelli previously requested changes Jul 30, 2024
View reviewed changes
Copy link
Contributor

@jakecastelli jakecastelli left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the contribution! The added doc is incorrect and this PR can be improved as the following:

  • blob.bytes() returns aPromise<Uint8Array> instead of Buffer
  • remove the unrelated file change as other reviewer pointed (remove test.txt)
  • please follow the commit message guide line where you can find it here in this case I would suggest commit message to be doc: add documentation for blob.bytes() method

Please let me know if you need any assistance!

jmsb02
jmsb02 commented Jul 30, 2024
View reviewed changes
Copy link
Contributor Author

@jmsb02 jmsb02 left a comment

Choose a reason for hiding this comment

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

Thank you for the feedback. I have made the following changes as per the suggestions:

  1. Removed the unrelated test.txt file.
  2. Updated the blob.bytes() method documentation to reflect the correct return type of Promise<Uint8Array>.

blob.bytes()

The blob.bytes() method returns the bytes of the Blob object as a Promise<Uint8Array>.

const blob = new Blob(['hello']);
blob.bytes().then((bytes) => {
  console.log(bytes); // Outputs: Uint8Array(5) [ 104, 101, 108, 108, 111 ]
});

Please let me know if there are any additional changes required. Thanks!

@jakecastelli
Copy link
Contributor

Do you mind squashing your 2 commits into 1 commit?

@jmsb02
doc: add documentation for blob.bytes() method
3a25717 
Add documentation for blob.bytes() method

doc: add documentation for blob.bytes() method
jmsb02
jmsb02 commented Jul 30, 2024
View reviewed changes
Copy link
Contributor Author

@jmsb02 jmsb02 left a comment

Choose a reason for hiding this comment

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

I have squashed the 2 commits into 1 commit as requested. Please let me know if there are any additional changes required. Thank you :)

@jakecastelli jakecastelli changed the title Issue #54105: Add documentation for blob.bytes() method doc: add documentation for blob.bytes() method Jul 30, 2024
jakecastelli
jakecastelli reviewed Jul 30, 2024
View reviewed changes
doc/api/buffer.md Outdated Show resolved Hide resolved
jmsb02
jmsb02 commented Jul 30, 2024
View reviewed changes
Copy link
Contributor Author

@jmsb02 jmsb02 left a comment

Choose a reason for hiding this comment

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

I have added the version information as you requested. Thank you for pointing that out. If there are any further changes needed, please let me know. I apologize for any inconvenience caused.

@jakecastelli jakecastelli mentioned this pull request Jul 30, 2024
Closed
@jakecastelli
Copy link
Contributor

No worries about the mistakes, you have been doing well! There is a pipeline failure due to linting, do you mind looking into it? 👀 Should be an easy fix!

jmsb02
jmsb02 commented Jul 30, 2024
View reviewed changes
Copy link
Contributor Author

@jmsb02 jmsb02 left a comment

Choose a reason for hiding this comment

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

I've just made the corrections you mentioned. As this is my first time, it's been challenging, but thanks to your help, I'm managing to get through it step by step. Thank you very much!

@jakecastelli
Copy link
Contributor

jakecastelli commented Jul 30, 2024 •
edited
Loading

Your last commit (198bfa6) was not correct, the one before that (6574a6a) is the correct linting fix. Could you drop the last commit (198bfa6)? As you can see your last commit had many irrelevant changes.

jmsb02
jmsb02 commented Jul 30, 2024
View reviewed changes
Copy link
Contributor Author

@jmsb02 jmsb02 left a comment

Choose a reason for hiding this comment

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

I have reset the branch to the correct commit (6574a6a) and force-pushed the changes. The incorrect commit (198bfa6) has been removed. Thank you for your guidance!

@jakecastelli jakecastelli dismissed their stale review July 30, 2024 15:26

Blockers have been addressed

daeyeon
daeyeon reviewed Jul 31, 2024
View reviewed changes
doc/api/buffer.md Outdated Show resolved Hide resolved
jmsb02
jmsb02 commented Jul 31, 2024
View reviewed changes
Copy link
Contributor Author

@jmsb02 jmsb02 left a comment

Choose a reason for hiding this comment

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

I have moved the blob.bytes() entry after blob.arrayBuffer so that it is in alphabetical order. Please let me know if there are any additional changes required. Thank you!

targos
targos reviewed Jul 31, 2024
View reviewed changes
doc/api/buffer.md Outdated
Comment on lines 558 to 559


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

targos
targos reviewed Jul 31, 2024
View reviewed changes
doc/api/buffer.md Outdated
});
```


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

targos
targos reviewed Jul 31, 2024
View reviewed changes
doc/api/buffer.md Outdated
Comment on lines 519 to 520
added:
- v20.16.0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
added:
- v20.16.0
added:
- v22.3.0
- v20.16.0

jmsb02
jmsb02 commented Jul 31, 2024
View reviewed changes
Copy link
Contributor Author

@jmsb02 jmsb02 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I have reflected your comments. Thank you!

@daeyeon daeyeon added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 3, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 3, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/54114
✔  Done loading data for nodejs/node/pull/54114
----------------------------------- PR info ------------------------------------
Title      doc: add documentation for blob.bytes() method  (#54114)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     jmsb02:add-blob-bytes-doc -> nodejs:main
Labels     author ready, commit-queue-squash
Commits    5
 - doc: add documentation for blob.bytes() method
 - doc: add documentation for blob.bytes() method
 - doc: add documentation for blob.bytes() method
 - doc: add documentation for blob.bytes() method
 - doc: add documentation for blob.bytes() method
Committers 1
 - jaexxin <jmsb02@naver.com>
PR-URL: https://github.com/nodejs/node/pull/54114
Fixes: https://github.com/nodejs/node/issues/54105
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/54114
Fixes: https://github.com/nodejs/node/issues/54105
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 30 Jul 2024 06:37:26 GMT
   ✔  Approvals: 1
   ✔  - Daeyeon Jeong (@daeyeon): https://github.com/nodejs/node/pull/54114#pullrequestreview-2217050112
   ✘  This PR needs to wait 65 more hours to land (or 0 hours if there is one more approval)
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/10228146599

@jakecastelli jakecastelli removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Aug 4, 2024
jakecastelli pushed a commit that referenced this pull request Aug 4, 2024
@jmsb02 @jakecastelli
doc: add documentation for blob.bytes() method
76d10a1 
PR-URL: #54114
Fixes: #54105
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
@jakecastelli
Copy link
Contributor

Landed in 76d10a1

@jakecastelli
Copy link
Contributor

Hi @jmsb02 I manually landed the PR for you as your first commit message body contains duplicated information. Congrats to become a contributor 🥳

@jakecastelli jakecastelli added the doc Issues and PRs related to the documentations. label Aug 4, 2024
targos pushed a commit that referenced this pull request Aug 14, 2024
@jmsb02 @targos
doc: add documentation for blob.bytes() method
11fdaa6 
PR-URL: #54114
Fixes: #54105
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Aug 19, 2024
Merged
@RedYetiDev RedYetiDev mentioned this pull request Aug 29, 2024
Closed
targos pushed a commit that referenced this pull request Sep 21, 2024
@jmsb02 @targos
doc: add documentation for blob.bytes() method
95b795f 
PR-URL: #54114
Fixes: #54105
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos left review comments

@pluris pluris pluris left review comments

@daeyeon daeyeon daeyeon approved these changes

@jakecastelli jakecastelli jakecastelli approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

blob.bytes() not documented
6 participants
@jmsb02 @jakecastelli @nodejs-github-bot @targos @daeyeon @pluris