Skip to content

Conversation

@kevinoid
Copy link
Contributor

The default for the emitClose option was changed from false to true by #31408 which landed in f0d2df4 for v14.0.0. This commit updates the fs doc to match the current behavior.

Thanks for considering,
Kevin

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Dec 28, 2020
@TrottTrott requested a review from ronagDecember 28, 2020 02:49
Copy link
Member

@ronagronag left a comment

Choose a reason for hiding this comment

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

I think it's better to just remove any mention of emitClose from the fs docs.

@kevinoid
Copy link
ContributorAuthor

I think it's better to just remove any mention of emitClose from the fs docs.

I agree with that sentiment, but I'm concerned that removing it from the docs would make existing code harder to understand, especially while supported versions still use emitClose: false by default.

I actually discovered this issue while checking the docs to understand why emitClose: true was being added to options in a call to fs.createWriteStream. If it had been removed, understanding would have been more difficult.

@kevinoidkevinoid requested a review from ronagDecember 28, 2020 14:20
The default for the `emitClose` option was changed from `false` to `true` by nodejs#31408 which landed in f0d2df4 for v14.0.0. This commit updates the fs doc to match the current behavior. Signed-off-by: Kevin Locke <[email protected]> PR-URL: nodejs#36653 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
@TrottTrottforce-pushed the for-upstream/fs-emitClose-docs branch from 19ca07b to d548f4dCompareJanuary 2, 2021 18:10
@TrottTrott merged commit d548f4d into nodejs:masterJan 2, 2021
@Trott
Copy link
Member

Trott commented Jan 2, 2021

Landed in d548f4d

danielleadams pushed a commit that referenced this pull request Jan 12, 2021
The default for the `emitClose` option was changed from `false` to `true` by #31408 which landed in f0d2df4 for v14.0.0. This commit updates the fs doc to match the current behavior. Signed-off-by: Kevin Locke <[email protected]> PR-URL: #36653 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
@danielleadamsdanielleadams mentioned this pull request Jan 12, 2021
targos pushed a commit that referenced this pull request Aug 8, 2021
The default for the `emitClose` option was changed from `false` to `true` by #31408 which landed in f0d2df4 for v14.0.0. This commit updates the fs doc to match the current behavior. Signed-off-by: Kevin Locke <[email protected]> PR-URL: #36653 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
BethGriggs pushed a commit that referenced this pull request Aug 12, 2021
The default for the `emitClose` option was changed from `false` to `true` by #31408 which landed in f0d2df4 for v14.0.0. This commit updates the fs doc to match the current behavior. Signed-off-by: Kevin Locke <[email protected]> PR-URL: #36653 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 31, 2021
The default for the `emitClose` option was changed from `false` to `true` by #31408 which landed in f0d2df4 for v14.0.0. This commit updates the fs doc to match the current behavior. Signed-off-by: Kevin Locke <[email protected]> PR-URL: #36653 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
@targostargos mentioned this pull request Sep 4, 2021
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
The default for the `emitClose` option was changed from `false` to `true` by nodejs#31408 which landed in f0d2df4 for v14.0.0. This commit updates the fs doc to match the current behavior. Signed-off-by: Kevin Locke <[email protected]> PR-URL: nodejs#36653 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.fsIssues and PRs related to the fs subsystem / file system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@kevinoid@Trott@ronag@targos@nodejs-github-bot