Skip to content

[x] fs: don't emit open after destroy - #28765

Closed
ronag wants to merge 1 commit into
nodejs:masterfrom
nxtedition:fix-fs-open-destroy
Closed

[x] fs: don't emit open after destroy#28765
ronag wants to merge 1 commit into
nodejs:masterfrom
nxtedition:fix-fs-open-destroy

Conversation

@ronag

@ronagronag commented Jul 19, 2019•
edited
Loading

Copy link
Copy Markdown
Member

Not perfect but still better than it was.

Refs: #23133

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 the fs Issues and PRs related to the fs subsystem / file system. label Jul 19, 2019
@ronag
ronagforce-pushed the fix-fs-open-destroy branch from d3fdc9b to ff9b6fcCompareJuly 19, 2019 14:21
mscdex
mscdex reviewed Jul 19, 2019
View reviewed changes
Comment threadlib/internal/fs/streams.js Outdated
@ronag
ronagforce-pushed the fix-fs-open-destroy branch 2 times, most recently from f404a6a to e7a8e22CompareJuly 19, 2019 17:51
@ronag

ronag commented Jul 19, 2019

Copy link
Copy Markdown
MemberAuthor

This might need a sem-ver major due to the updated test.

@ronag
ronagforce-pushed the fix-fs-open-destroy branch from e7a8e22 to 281772eCompareJuly 19, 2019 17:52
@Trott

Trott commented Jul 22, 2019

Copy link
Copy Markdown
Member

@nodejs/fs

@jasnelljasnell added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jul 29, 2019
@trivikr
trivikr requested a review from mscdexJuly 29, 2019 20:07
@ronag

ronag commented Aug 2, 2019

Copy link
Copy Markdown
MemberAuthor

@Trott author ready?

@Trott

Trott commented Aug 2, 2019

Copy link
Copy Markdown
Member

@Trott author ready?

It's semver-major and all (or at least many) semver-major PRs are blocked on standard-things/esm#821. 😞

@nodejs-github-bot

nodejs-github-bot commented Aug 2, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/24824/

This was referenced Aug 6, 2019
Closed
Closed
@ronag

ronag commented Aug 25, 2019•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Alternative solution which is further into the future nxtedition#2.

@ronag

ronag commented Aug 28, 2019•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

closing for now, there are other PR's that solves this better, but with bigger changes.

@ronagronag closed this Aug 28, 2019
@ronagronag changed the title fs: don't emit open after destroy[x] fs: don't emit open after destroyAug 28, 2019
@fsouzafsouza mentioned this pull request Nov 19, 2019
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@mscdexmscdexAwaiting requested review from mscdex

Assignees

No one assigned

Labels

fsIssues and PRs related to the fs subsystem / file system.semver-majorPRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ronag@Trott@nodejs-github-bot@mscdex@jasnell