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: add fsPromises.readFile() example #40237

Merged

Conversation

bnb
Copy link
Contributor

@bnb bnb commented Sep 27, 2021

Adds a CJS and MJS example to the readFile method on fsPromises. There is an example with AbortController but it's slightly misleading if you're just looking at the examples, since you don't actually need to do that.

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Sep 27, 2021
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

I wonder if an example showing how to read a package.json relative to the current module wouldn't be more common than from an absolute path.

doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
@bnb
Copy link
Contributor Author

bnb commented Oct 20, 2021

Would love a second opinion on whether or not we'd prefer to use path in this code example. Personally I like to try to stay away from layering on additional concepts for examples, but if it's the preferred approach of the project I'd be happy to do it.

@mhdawson
Copy link
Member

I see path.join is already used in the same file for other examples. I don't have a strong preference but I would lean towards using the path.join version suggested.

@bnb
Copy link
Contributor Author

bnb commented Oct 20, 2021

(going to pull and make some small tweaks)

Signed-off-by: Tierney Cyren <hello@bnb.im>
@bnb bnb force-pushed the bnb/add-fspromises-readfile-example branch from 39ec877 to a580c73 Compare November 15, 2021 22:23
@bnb
Copy link
Contributor Author

bnb commented Nov 15, 2021

force pushed to fix prefix issues. hopefully mergable now :)

@bnb
Copy link
Contributor Author

bnb commented Nov 16, 2021

it looks like this line is over 80 characters. Do we block on that for code examples?

doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
bnb and others added 3 commits October 10, 2022 19:03
Co-authored-by: Rich Trott <rtrott@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
bnb and others added 3 commits October 11, 2022 00:03
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 11, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 11, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/40237
✔  Done loading data for nodejs/node/pull/40237
----------------------------------- PR info ------------------------------------
Title      doc: add fsPromises.readFile() example (#40237)
Author     Tierney Cyren  (@bnb)
Branch     bnb:bnb/add-fspromises-readfile-example -> nodejs:main
Labels     doc, fs
Commits    7
 - doc: add fsPromises.readFile() example
 - doc: simplify code example
 - doc: simplify code example
 - doc: remove unnecessary line break
 - doc: fix line length issue
 - doc: use `node:` prefix for node modules
 - doc: use `node:` prefix for node modules
Committers 2
 - Tierney Cyren 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/40237
Reviewed-By: James M Snell 
Reviewed-By: Antoine du Hamel 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/40237
Reviewed-By: James M Snell 
Reviewed-By: Antoine du Hamel 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 27 Sep 2021 23:22:52 GMT
   ✔  Approvals: 2
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/40237#pullrequestreview-787530318
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/40237#pullrequestreview-1136672708
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 40237
From https://github.com/nodejs/node
 * branch                  refs/pull/40237/merge -> FETCH_HEAD
✔  Fetched commits as 1124558b655c..90a77178015a
--------------------------------------------------------------------------------
Auto-merging doc/api/fs.md
[main 5a063004ea] doc: add fsPromises.readFile() example
 Author: Tierney Cyren 
 Date: Mon Nov 15 17:22:46 2021 -0500
 1 file changed, 26 insertions(+)
Auto-merging doc/api/fs.md
[main c10f6c3798] doc: simplify code example
 Author: Tierney Cyren 
 Date: Mon Oct 10 19:03:27 2022 +0200
 1 file changed, 2 insertions(+), 1 deletion(-)
Auto-merging doc/api/fs.md
[main 92f98b36c4] doc: simplify code example
 Author: Tierney Cyren 
 Date: Mon Oct 10 19:03:44 2022 +0200
 1 file changed, 2 insertions(+), 1 deletion(-)
Auto-merging doc/api/fs.md
[main 40cef2f085] doc: remove unnecessary line break
 Author: Tierney Cyren 
 Date: Mon Oct 10 19:03:59 2022 +0200
 1 file changed, 1 deletion(-)
Auto-merging doc/api/fs.md
[main 496078fb77] doc: fix line length issue
 Author: Tierney Cyren 
 Date: Tue Oct 11 00:03:02 2022 +0200
 1 file changed, 3 insertions(+), 1 deletion(-)
Auto-merging doc/api/fs.md
[main 366bef88c7] doc: use `node:` prefix for node modules
 Author: Tierney Cyren 
 Date: Tue Oct 11 00:03:25 2022 +0200
 1 file changed, 1 insertion(+), 1 deletion(-)
Auto-merging doc/api/fs.md
[main 19a6ba1076] doc: use `node:` prefix for node modules
 Author: Tierney Cyren 
 Date: Tue Oct 11 00:03:35 2022 +0200
 1 file changed, 2 insertions(+), 2 deletions(-)
   ✔  Patches applied
There are 7 commits in the PR. Attempting autorebase.
Rebasing (2/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: add fsPromises.readFile() example

Signed-off-by: Tierney Cyren hello@bnb.im
PR-URL: #40237
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com

[detached HEAD c0e2cc46ab] doc: add fsPromises.readFile() example
Author: Tierney Cyren hello@bnb.im
Date: Mon Nov 15 17:22:46 2021 -0500
1 file changed, 26 insertions(+)
Rebasing (3/14)
Rebasing (4/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: simplify code example

Co-authored-by: Rich Trott rtrott@gmail.com
PR-URL: #40237
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com

[detached HEAD cead95bac5] doc: simplify code example
Author: Tierney Cyren accounts@bnb.im
Date: Mon Oct 10 19:03:27 2022 +0200
1 file changed, 2 insertions(+), 1 deletion(-)
Rebasing (5/14)
Rebasing (6/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: simplify code example

Co-authored-by: Rich Trott rtrott@gmail.com
PR-URL: #40237
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com

[detached HEAD 054aac7cd5] doc: simplify code example
Author: Tierney Cyren accounts@bnb.im
Date: Mon Oct 10 19:03:44 2022 +0200
1 file changed, 2 insertions(+), 1 deletion(-)
Rebasing (7/14)
Rebasing (8/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: remove unnecessary line break

Co-authored-by: Rich Trott rtrott@gmail.com
PR-URL: #40237
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com

[detached HEAD 0e0e2ec4cd] doc: remove unnecessary line break
Author: Tierney Cyren accounts@bnb.im
Date: Mon Oct 10 19:03:59 2022 +0200
1 file changed, 1 deletion(-)
Rebasing (9/14)
Rebasing (10/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: fix line length issue

Co-authored-by: Antoine du Hamel duhamelantoine1995@gmail.com
PR-URL: #40237
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com

[detached HEAD ccf76fe1ae] doc: fix line length issue
Author: Tierney Cyren accounts@bnb.im
Date: Tue Oct 11 00:03:02 2022 +0200
1 file changed, 3 insertions(+), 1 deletion(-)
Rebasing (11/14)
Rebasing (12/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: use node: prefix for node modules

Co-authored-by: Antoine du Hamel duhamelantoine1995@gmail.com
PR-URL: #40237
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com

[detached HEAD 364aa384c5] doc: use node: prefix for node modules
Author: Tierney Cyren accounts@bnb.im
Date: Tue Oct 11 00:03:25 2022 +0200
1 file changed, 1 insertion(+), 1 deletion(-)
Rebasing (13/14)
Rebasing (14/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: use node: prefix for node modules

Co-authored-by: Antoine du Hamel duhamelantoine1995@gmail.com
PR-URL: #40237
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com

[detached HEAD 256f3fc2f2] doc: use node: prefix for node modules
Author: Tierney Cyren accounts@bnb.im
Date: Tue Oct 11 00:03:35 2022 +0200
1 file changed, 2 insertions(+), 2 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/3223732038

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 11, 2022
@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 11, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 11, 2022
@nodejs-github-bot nodejs-github-bot merged commit 624dadb into nodejs:main Oct 11, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 624dadb

danielleadams pushed a commit that referenced this pull request Oct 11, 2022
Signed-off-by: Tierney Cyren <hello@bnb.im>
PR-URL: #40237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants