FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

stream: make async iteration stable by mcollina · Pull Request #26989 · nodejs/node · GitHub

/ node Public

stream: make async iteration stable - #26989

Closed
mcollina wants to merge 2 commits into
nodejs:masterfrom
mcollina:make-readable-async-iterator-stable
ConversationCommits (2)ChecksFiles changed
Closed

stream: make async iteration stable#26989
mcollina wants to merge 2 commits into
nodejs:masterfrom
mcollina:make-readable-async-iterator-stable

Conversation

mcollina commented Mar 29, 2019

Copy link
Copy Markdown
Member

I think it's about time we make Readable async iteration stable.
It has been around for a while - we have fixed a bunch of bugs so far, and I expect there will still be some. I do not expect major changes.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

mcollina added stream Issues and PRs related to the stream subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Mar 29, 2019
mcollina requested review from a team and jasnell March 29, 2019 16:17

nodejs-github-bot commented Mar 29, 2019

Copy link
Copy Markdown
Collaborator

@mcollina Sadly, an error occurred when I tried to trigger a build. :(

mcollina mentioned this pull request Mar 29, 2019
Closed
cjihrig approved these changes Mar 29, 2019
View reviewed changes
Comment thread doc/api/stream.md Outdated

vsemozhetbyt commented Mar 29, 2019

Copy link
Copy Markdown
Contributor

Is it worth to do the same for rl[Symbol.asyncIterator]()?

vsemozhetbyt added the promises Issues and PRs related to ECMAScript promises. label Mar 29, 2019

targos commented Mar 29, 2019

Copy link
Copy Markdown
Member

why semver-major?

mcollina commented Mar 29, 2019

Copy link
Copy Markdown
Member Author

why semver-major?

I don't know really.

mcollina commented Mar 29, 2019

Copy link
Copy Markdown
Member Author

Is it worth to do the same for rlSymbol.asyncIterator?

I'm happy to include that in this PR.

mcollina added semver-minor PRs that contain new features and should be released in the next minor version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels Mar 29, 2019
jasnell reviewed Mar 29, 2019
View reviewed changes
Comment thread doc/api/stream.md Outdated

jasnell Mar 29, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
description: AsyncIterator support is not experimental anymore.
description: AsyncIterator support is no longer experimental.

BridgeAR added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed author ready PRs that have at least one approval, no outstanding review comments, and a CI started. labels Mar 31, 2019
benjamingr approved these changes Mar 31, 2019
View reviewed changes

benjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

🎉

mcollina force-pushed the make-readable-async-iterator-stable branch from 9b5ab18 to 8df8328 Compare April 1, 2019 08:09

mcollina commented Apr 1, 2019

Copy link
Copy Markdown
Member Author

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

1 similar comment

nodejs-github-bot commented Apr 1, 2019

Copy link
Copy Markdown
Collaborator

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

mcollina commented Apr 1, 2019

Copy link
Copy Markdown
Member Author

I've also marked readline AsyncIterator support stable.

PTAL @vsemozhetbyt @jasnell @addaleax @benjamingr @cjihrig @BridgeAR @shisama

benjamingr approved these changes Apr 1, 2019
View reviewed changes

benjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Still LGTM

BethGriggs pushed a commit that referenced this pull request Apr 9, 2019
stream: make Symbol.asyncIterator support stable
b29afa2 
PR-URL: #26989
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 9, 2019
readline: make Symbol.asyncIterator support stable
dbd0608 
PR-URL: #26989
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BethGriggs mentioned this pull request Apr 9, 2019
Merged
BethGriggs pushed a commit that referenced this pull request Apr 10, 2019
stream: make Symbol.asyncIterator support stable
0ac41fc 
PR-URL: #26989
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 10, 2019
readline: make Symbol.asyncIterator support stable
6d39460 
PR-URL: #26989
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BethGriggs added a commit that referenced this pull request Apr 10, 2019
2019-04-10, Version 11.14.0 (Current)
ebfb67a 
Notable changes:

- child_process: doc deprecate ChildProcess.\_channel (cjihrig)
  [#26982](#26982)
- deps: update nghttp2 to 1.37.0 (gengjiawen)
  [#26990](#26990)
- dns:
  - make dns.promises enumerable (cjihrig)
    [#26592](#26592)
  - remove dns.promises experimental warning (cjihrig)
    [#26592](#26592)
- stream: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)
- worker: use copy of process.env (Anna Henningsen)
  [#26544](#26544)

PR-URL: #27163
BethGriggs added a commit that referenced this pull request Apr 11, 2019
2019-04-11, Version 11.14.0 (Current)
cd026f8 
Notable changes:

- child_process: doc deprecate ChildProcess.\_channel (cjihrig)
  [#26982](#26982)
- deps: update nghttp2 to 1.37.0 (gengjiawen)
  [#26990](#26990)
- dns:
  - make dns.promises enumerable (cjihrig)
    [#26592](#26592)
  - remove dns.promises experimental warning (cjihrig)
    [#26592](#26592)
- fs: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581] (#26581)
- stream: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)
- worker: use copy of process.env (Anna Henningsen)
  [#26544](#26544)

PR-URL: #27163
BethGriggs added a commit that referenced this pull request Apr 11, 2019
2019-04-11, Version 11.14.0 (Current)
d424560 
Notable changes:

- child_process: doc deprecate ChildProcess.\_channel (cjihrig)
  [#26982](#26982)
- deps: update nghttp2 to 1.37.0 (gengjiawen)
  [#26990](#26990)
- dns:
  - make dns.promises enumerable (cjihrig)
    [#26592](#26592)
  - remove dns.promises experimental warning (cjihrig)
    [#26592](#26592)
- fs: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581] (#26581)
- stream: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)
- worker: use copy of process.env (Anna Henningsen)
  [#26544](#26544)

PR-URL: #27163
cjihrig mentioned this pull request May 2, 2019
Closed

mcollina commented May 19, 2019

Copy link
Copy Markdown
Member Author

@nodejs/lts would you mind including this in the next v10 minor?

vschoener commented May 21, 2019

Copy link
Copy Markdown

That would be awesome :)

mcollina commented Sep 4, 2019

Copy link
Copy Markdown
Member Author

@BethGriggs would you mind adding this to the next 10 release?

BethGriggs pushed a commit that referenced this pull request Oct 18, 2019
stream: make Symbol.asyncIterator support stable
00831f0 
PR-URL: #26989
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BethGriggs pushed a commit that referenced this pull request Oct 18, 2019
readline: make Symbol.asyncIterator support stable
79f3844 
PR-URL: #26989
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BethGriggs added a commit that referenced this pull request Oct 18, 2019
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
44ef30e 
Notable changes:

- **deps**: upgrade openssl sources to 1.1.1d (Sam Roberts)
  [#29921](#29921)
- **dns**: remove dns.promises experimental warning (cjihrig)
  [#26592](#26592)
- **fs**: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581](#26581)
- **n-api**: mark version 5 N-APIs as stable (Gabriel Schulhof)
  [#29401](#29401)
- **stream**: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)

PR-URL: #29875
BethGriggs mentioned this pull request Oct 18, 2019
Merged
BethGriggs added a commit that referenced this pull request Oct 19, 2019
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
c3e596f 
Notable changes:

- **deps**: update npm to 6.11.3 (claudiahdz)
  [#29430](#29430)
- **deps**: upgrade openssl sources to 1.1.1d (Sam Roberts)
  [#29921](#29921)
- **dns**: remove dns.promises experimental warning (cjihrig)
  [#26592](#26592)
- **fs**: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581](#26581)
- **n-api**: mark version 5 N-APIs as stable (Gabriel Schulhof)
  [#29401](#29401)
- **stream**: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)

PR-URL: #29875
BethGriggs added a commit that referenced this pull request Oct 21, 2019
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
ae884d3 
Notable changes:

- **deps**: update npm to 6.11.3 (claudiahdz)
  [#29430](#29430)
- **deps**: upgrade openssl sources to 1.1.1d (Sam Roberts)
  [#29921](#29921)
- **dns**: remove dns.promises experimental warning (cjihrig)
  [#26592](#26592)
- **fs**: remove experimental warning for fs.promises (Anna Henningsen)
  [#26581](#26581)
- **n-api**: mark version 5 N-APIs as stable (Gabriel Schulhof)
  [#29401](#29401)
- **stream**: make Symbol.asyncIterator support stable (Matteo Collina)
  [#26989](#26989)

PR-URL: #29875
BethGriggs added a commit that referenced this pull request Oct 22, 2019
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
5dae3ef 
Notable changes:

* crypto:
  * add support for chacha20-poly1305 for AEAD (chux0519)
    #24081
  * increase maxmem range from 32 to 53 bits (Tobias Nießen)
    #28799
* deps:
  * update npm to 6.11.3 (claudiahdz)
    #29430
  * upgrade openssl sources to 1.1.1d (Sam Roberts)
    #29921
* dns:
  * remove dns.promises experimental warning (cjihrig)
    #26592
* fs:
  * remove experimental warning for fs.promises (Anna Henningsen)
    #26581
* http:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* http2:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* n-api:
  * make func argument of napi\_create\_threadsafe\_function optional
    (legendecas)
    #27791
  * mark version 5 N-APIs as stable (Gabriel Schulhof)
    #29401
  * implement date object (Jarrod Connolly)
    #25917
* process:
  * add --unhandled-rejections flag (Ruben Bridgewater)
    #26599
* stream:
  * implement Readable.from async iterator utility (Guy Bedford)
    #27660
  * make Symbol.asyncIterator support stable (Matteo Collina)
    #26989

PR-URL: #29875
BethGriggs added a commit that referenced this pull request Oct 22, 2019
2019-10-22, Version 10.17.0 'Dubnium' (LTS)
afbbcb0 
Notable changes:

* crypto:
  * add support for chacha20-poly1305 for AEAD (chux0519)
    #24081
  * increase maxmem range from 32 to 53 bits (Tobias Nießen)
    #28799
* deps:
  * update npm to 6.11.3 (claudiahdz)
    #29430
  * upgrade openssl sources to 1.1.1d (Sam Roberts)
    #29921
* dns:
  * remove dns.promises experimental warning (cjihrig)
    #26592
* fs:
  * remove experimental warning for fs.promises (Anna Henningsen)
    #26581
* http:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* http2:
  * makes response.writeHead return the response (Mark S. Everitt)
    #25974
* n-api:
  * make func argument of napi\_create\_threadsafe\_function optional
    (legendecas)
    #27791
  * mark version 5 N-APIs as stable (Gabriel Schulhof)
    #29401
  * implement date object (Jarrod Connolly)
    #25917
* process:
  * add --unhandled-rejections flag (Ruben Bridgewater)
    #26599
* stream:
  * implement Readable.from async iterator utility (Guy Bedford)
    #27660
  * make Symbol.asyncIterator support stable (Matteo Collina)
    #26989

PR-URL: #29875
jepstein-eagle mentioned this pull request Jun 11, 2020
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
addaleax addaleax approved these changes
benjamingr benjamingr approved these changes
cjihrig cjihrig approved these changes
BridgeAR BridgeAR approved these changes
+1 more reviewer
shisama shisama approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. promises Issues and PRs related to ECMAScript promises. semver-minor PRs that contain new features and should be released in the next minor version. 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.

Back | FazBrowse Home | New Git URL