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: remove redundant only #14858

Closed
wants to merge 1 commit into from
Closed

doc: remove redundant only #14858

wants to merge 1 commit into from

Conversation

GeorgeSapkin
Copy link
Contributor

Checklist
Affected core subsystem(s)

doc

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Aug 16, 2017
@@ -1448,8 +1448,8 @@ added: v8.0.0
argument) when you are done writing any remaining data.

Note: `_final()` **must not** be called directly. It MAY be implemented
by child classes, and if so, will be called by the internal Writable
class methods only.
by child classes, and if so, will be called by the internal Writable class
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated/unnecessary change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I remove it?

Copy link
Member

Choose a reason for hiding this comment

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

@GeorgeSapkin Yes, please. We like to keep diffs as small as possible as long as there is no noticeable benefit of having such changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@mscdex
Copy link
Contributor

mscdex commented Aug 16, 2017

LGTM

Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

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

LGTM with #14858 (comment) addressed.

@tniessen tniessen self-assigned this Aug 16, 2017
@tniessen
Copy link
Member

Landed in 4cff2cc! 🎉 Thank you for your first contribution!

@tniessen tniessen closed this Aug 20, 2017
tniessen pushed a commit that referenced this pull request Aug 20, 2017
PR-URL: #14858
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@GeorgeSapkin GeorgeSapkin deleted the fix-stream-docs branch August 20, 2017 10:08
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
PR-URL: #14858
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
PR-URL: #14858
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants