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

fs: use _final() for fs.WriteStream #20562

Closed
wants to merge 1 commit into from
Closed

fs: use _final() for fs.WriteStream #20562

wants to merge 1 commit into from

Conversation

JacksonTian
Copy link
Contributor

@JacksonTian JacksonTian commented May 6, 2018 •
edited
Loading

use _final() method instead of once 'finish' event.

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

@JacksonTian
fs: use _final() for fs.WriteStream
2003b3d 
use _final() method instead of once 'finish' event.
@nodejs-github-bot nodejs-github-bot added the fs Issues and PRs related to the fs subsystem / file system. label May 6, 2018
@ChALkeR ChALkeR added the stream Issues and PRs related to the stream subsystem. label May 6, 2018
@lpinca
Copy link
Member

lpinca commented May 6, 2018

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

addaleax
addaleax approved these changes May 6, 2018
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@lpinca lpinca added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 12, 2018
@addaleax
Copy link
Member

Landed in 3746187

@addaleax addaleax closed this May 14, 2018
addaleax pushed a commit that referenced this pull request May 14, 2018
@JacksonTian @addaleax
fs: use _final() for fs.WriteStream
3746187 
use _final() method instead of once 'finish' event.

PR-URL: #20562
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request May 14, 2018
@JacksonTian @addaleax
fs: use _final() for fs.WriteStream
39caa6d 
use _final() method instead of once 'finish' event.

PR-URL: #20562
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax addaleax mentioned this pull request May 14, 2018
Merged
@JacksonTian JacksonTian deleted the final branch May 15, 2018 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@lpinca lpinca lpinca approved these changes

@trivikr trivikr trivikr 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. fs Issues and PRs related to the fs subsystem / file system. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@JacksonTian @lpinca @addaleax @jasnell @trivikr @ChALkeR @nodejs-github-bot