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

docs: update options in filehandle.appendFile() #56972

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

y-hsgw
Copy link

@y-hsgw y-hsgw commented Feb 9, 2025

I updated the documentation for the filehandle.appendFile() options.
And I also added related tests.

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Feb 9, 2025
doc/api/fs.md Show resolved Hide resolved
await assert.rejects(fileHandle.appendFile(buffer, { signal }), {
name: 'AbortError'
});
} finally {
Copy link
Member

Choose a reason for hiding this comment

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

The try...catch is not needed as the process exits when an error occurs.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I removed in 78c5b07.

validateAppendBuffer(),
validateAppendString(),
doAppendAndCancel(),
]).then(common.mustCall());
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
]).then(common.mustCall());
]);

This is no longer needed. By default, an unhandled promise rejection makes the process exit with a non-zero exit code.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I fixed in 27ae1f8.

@lpinca
Copy link
Member

lpinca commented Feb 10, 2025

Please replace docs: with doc: in the second commit.

@y-hsgw y-hsgw force-pushed the docs/fs-file-handle-append-file branch from d40ef79 to 47fb2d6 Compare February 10, 2025 14:09
Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.13%. Comparing base (b181535) to head (78c5b07).
Report is 13 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #56972   +/-   ##
=======================================
  Coverage   89.13%   89.13%           
=======================================
  Files         665      665           
  Lines      193165   193174    +9     
  Branches    37191    37193    +2     
=======================================
+ Hits       172181   172193   +12     
- Misses      13729    13740   +11     
+ Partials     7255     7241   -14     

see 26 files with indirect coverage changes

@lpinca
Copy link
Member

lpinca commented Feb 10, 2025

Can you please rebase and foce push to keep only two commits? Thank you.

@y-hsgw y-hsgw force-pushed the docs/fs-file-handle-append-file branch from 78c5b07 to 42595e2 Compare February 11, 2025 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants