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: add links between cork() and uncork() #11222

Closed
wants to merge 1 commit into from

Conversation

mcollina
Copy link
Member

@mcollina mcollina commented Feb 7, 2017

Writable.cork() and Writable.uncork() are meant to be documented
together, but we maintain a lexicographic order. This commit
introduces some links between the two.

Fixes: #7340

Implements @addaleax suggestion in #7340 (comment)

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc, stream

@mcollina mcollina added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Feb 7, 2017
@mcollina mcollina requested a review from addaleax February 7, 2017 18:13
@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 Feb 7, 2017
@@ -347,6 +347,8 @@ buffer that would have an adverse impact on performance. In such situations,
implementations that implement the `writable._writev()` method can perform
buffered writes in a more optimized manner.

See also: [`writable.uncork()`][].
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work without defining cork() and uncork() in the links at the bottom of the page?

Copy link
Member Author

Choose a reason for hiding this comment

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

It did not. Now it does.

Copy link
Contributor

@cjihrig cjihrig 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 one comment.

Copy link
Contributor

@Fishrock123 Fishrock123 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 @cjihrig's comment

Writable.cork() and Writable.uncork() are meant to be documented
together, but we maintain a lexicographic order. This commit
introduces some links between the two.

Fixes: nodejs#7340
@mcollina
Copy link
Member Author

mcollina commented Feb 7, 2017

Updated with @cjihrig and @Fishrock123 suggestion.

@mcollina
Copy link
Member Author

mcollina commented Feb 9, 2017

Landed as c38b6d2

@mcollina mcollina closed this Feb 9, 2017
mcollina added a commit that referenced this pull request Feb 9, 2017
Writable.cork() and Writable.uncork() are meant to be documented
together, but we maintain a lexicographic order. This commit
introduces some links between the two.

Fixes: #7340
PR-URL: #11222
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 9, 2017
Writable.cork() and Writable.uncork() are meant to be documented
together, but we maintain a lexicographic order. This commit
introduces some links between the two.

Fixes: nodejs#7340
PR-URL: nodejs#11222
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 14, 2017
Writable.cork() and Writable.uncork() are meant to be documented
together, but we maintain a lexicographic order. This commit
introduces some links between the two.

Fixes: nodejs#7340
PR-URL: nodejs#11222
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
krydos pushed a commit to krydos/node that referenced this pull request Feb 25, 2017
Writable.cork() and Writable.uncork() are meant to be documented
together, but we maintain a lexicographic order. This commit
introduces some links between the two.

Fixes: nodejs#7340
PR-URL: nodejs#11222
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell
Copy link
Member

jasnell commented Mar 7, 2017

needs a backport PR to land on v4

jasnell pushed a commit that referenced this pull request Mar 7, 2017
Writable.cork() and Writable.uncork() are meant to be documented
together, but we maintain a lexicographic order. This commit
introduces some links between the two.

Fixes: #7340
PR-URL: #11222
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@mcollina
Copy link
Member Author

mcollina commented Mar 7, 2017

Do we need to backport?

@jasnell
Copy link
Member

jasnell commented Mar 7, 2017

not necessarily. just indicating that a backport would be needed if we did want to land it in those branches

MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
Writable.cork() and Writable.uncork() are meant to be documented
together, but we maintain a lexicographic order. This commit
introduces some links between the two.

Fixes: #7340
PR-URL: #11222
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
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.

stream docs, writable.cork() and writable.uncork() methods
6 participants