stream: writableCorked by ronag · Pull Request #29012 · nodejs/node

ronag · 2019-08-06T11:41:42Z

Expose _writableState.corked.

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

ronag · 2019-08-06T11:41:49Z

Does this need a test?

addaleax · 2019-08-06T11:47:20Z

Does this need a test?

Yes, almost all PRs that touch lib/ or src/ should have tests, if possible. And in particular, new APIs should always come with tests.

ronag · 2019-08-06T11:51:00Z

Test added

doc/api/stream.md

test/parallel/test-stream-writable-properties.js

mcollina

LGTM

@jasnell is this ok now?

mcollina · 2019-08-28T07:39:59Z

This needs a rebase.

ronag · 2019-08-28T07:51:45Z

rebased

nodejs-github-bot · 2019-08-28T08:30:04Z

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

mcollina · 2019-09-02T15:13:30Z

There were quite a few CI failures, would you mind taking a look?

ronag · 2019-09-02T17:02:42Z

another rebase

ronag · 2019-09-02T18:40:56Z

@Trott: Travis problems?

22760.2 Compile V8 (py3) | errored
-- | --

and then everything else is cancelled...

addaleax

Still LGTM

nodejs-github-bot · 2019-11-05T18:28:37Z

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

addaleax · 2019-11-05T22:28:44Z

Landed in de11913

PR-URL: #29012 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

BridgeAR · 2019-11-19T17:34:11Z

@mcollina @jasnell @addaleax @ronag before we publish this in the next release: is the name what we want to stick with? .cork() and .uncork() are only available for writable streams, so maybe .corked as property name would have been sufficient? Just asking as I noticed the redundancy while putting together the changelog for v13.2.0.

mcollina · 2019-11-19T17:39:57Z

@BridgeAR I think we should stick with the prefix for consistency. I'm also fine in dropping it.

addaleax · 2019-11-19T18:27:03Z

I also don’t have a strong opinion on the name.

ronag · 2019-11-19T20:03:33Z

I also think we should stick with consistency. We've already postfixed all the other writable specific properties. Changing the convention now feels unnecessary and unintuitive. I feel rather strongly about this.

Notable changes: * addons: * Deprecate one- and two-argument `AtExit()`. Use the three-argument variant of `AtExit()` or `AddEnvironmentCleanupHook()` instead (Anna Henningsen) nodejs#30227 * child_process,cluster: * The `serialization` option is added that allows child process IPC to use the V8 serialization API (to e.g., pass through data types like sets or maps) (Anna Henningsen) nodejs#30162 * deps: * Update V8 to 7.9 * Update `npm` to 6.13.0 (Ruy Adorno) nodejs#30271 * embedder: * Exposes the ability to pass cli flags / options through an API as embedder (Shelley Vohr) nodejs#30466 * Allow adding linked bindings to Environment (Anna Henningsen) nodejs#30274 * esm: * Unflag --experimental-modules (Guy Bedford) nodejs#29866 * stream: * Add `writable.writableCorked` property (Robert Nagy) nodejs#29012 * worker: * Allow specifying resource limits (Anna Henningsen) nodejs#26628 * v8: * The Serialization API is now stable (Anna Henningsen) nodejs#30234 PR-URL: nodejs#30547

Notable changes: * addons: * Deprecate one- and two-argument `AtExit()`. Use the three-argument variant of `AtExit()` or `AddEnvironmentCleanupHook()` instead (Anna Henningsen) #30227 * child_process,cluster: * The `serialization` option is added that allows child process IPC to use the V8 serialization API (to e.g., pass through data types like sets or maps) (Anna Henningsen) #30162 * deps: * Update V8 to 7.9 * Update `npm` to 6.13.0 (Ruy Adorno) #30271 * embedder: * Exposes the ability to pass cli flags / options through an API as embedder (Shelley Vohr) #30466 * Allow adding linked bindings to Environment (Anna Henningsen) #30274 * esm: * Unflag --experimental-modules (Guy Bedford) #29866 * stream: * Add `writable.writableCorked` property (Robert Nagy) #29012 * worker: * Allow specifying resource limits (Anna Henningsen) #26628 * v8: * The Serialization API is now stable (Anna Henningsen) #30234 PR-URL: #30547

PR-URL: #29012 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

nodejs-github-bot added http Issues or PRs related to the http subsystem. stream Issues and PRs related to the stream subsystem. labels Aug 6, 2019

ronag mentioned this pull request Aug 6, 2019

stream: internal vs external API and streamlike compat #29006

Closed

8 tasks

ronag force-pushed the stream-writablecorked branch from 9d143cc to 268c091 Compare August 6, 2019 11:44

addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 6, 2019

ronag force-pushed the stream-writablecorked branch from 268c091 to e49d6a4 Compare August 6, 2019 11:50

ronag force-pushed the stream-writablecorked branch 2 times, most recently from 631ebb7 to 8358cb8 Compare August 6, 2019 11:52

addaleax approved these changes Aug 6, 2019

View reviewed changes

vsemozhetbyt reviewed Aug 6, 2019

View reviewed changes

doc/api/stream.md Outdated Show resolved Hide resolved

ronag force-pushed the stream-writablecorked branch from 8358cb8 to f7fe3cf Compare August 6, 2019 13:03

ronag mentioned this pull request Aug 6, 2019

http: OutgoingMessage streamlike #28969

Closed

9 tasks

ronag force-pushed the stream-writablecorked branch 2 times, most recently from e182b30 to e37e92e Compare August 6, 2019 20:32

jasnell requested changes Aug 7, 2019

View reviewed changes

test/parallel/test-stream-writable-properties.js Outdated Show resolved Hide resolved

ronag force-pushed the stream-writablecorked branch 2 times, most recently from 41713d2 to 80cec35 Compare August 9, 2019 16:03

mcollina approved these changes Aug 28, 2019

View reviewed changes

ronag force-pushed the stream-writablecorked branch from 80cec35 to dca2160 Compare August 28, 2019 07:51

stream: writableCorked

67dae03

ronag force-pushed the stream-writablecorked branch from dca2160 to 67dae03 Compare September 2, 2019 17:02

ronag mentioned this pull request Nov 4, 2019

http: outgoing cork #29053

Closed

4 tasks

addaleax approved these changes Nov 5, 2019

View reviewed changes

addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 5, 2019

addaleax closed this Nov 5, 2019

addaleax pushed a commit that referenced this pull request Nov 5, 2019

stream: add writableCorked property

de11913

PR-URL: #29012 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

MylesBorins pushed a commit that referenced this pull request Nov 17, 2019

stream: add writableCorked property

c254d74

PR-URL: #29012 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

BridgeAR mentioned this pull request Nov 19, 2019

v13.2.0 proposal #30547

Merged

targos pushed a commit that referenced this pull request Jan 13, 2020

stream: add writableCorked property

6a8a33c

PR-URL: #29012 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

targos mentioned this pull request Jan 15, 2020

v12.15.0 release proposal #31368

Closed

BethGriggs pushed a commit that referenced this pull request Feb 6, 2020

stream: add writableCorked property

af960d7

PR-URL: #29012 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

MylesBorins mentioned this pull request Feb 8, 2020

v12.16.0 proposal #31691

Merged

Uh oh!

Conversation

ronag commented Aug 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

ronag commented Aug 6, 2019

Uh oh!

addaleax commented Aug 6, 2019

Uh oh!

ronag commented Aug 6, 2019

Uh oh!

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina commented Aug 28, 2019

Uh oh!

ronag commented Aug 28, 2019

Uh oh!

nodejs-github-bot commented Aug 28, 2019

Uh oh!

mcollina commented Sep 2, 2019

Uh oh!

ronag commented Sep 2, 2019

Uh oh!

ronag commented Sep 2, 2019

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Nov 5, 2019

Uh oh!

addaleax commented Nov 5, 2019

Uh oh!

BridgeAR commented Nov 19, 2019

Uh oh!

mcollina commented Nov 19, 2019

Uh oh!

addaleax commented Nov 19, 2019

Uh oh!

ronag commented Nov 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ronag commented Aug 6, 2019 •

edited

Loading

ronag commented Nov 19, 2019 •

edited

Loading