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

stream: make async iteration stable #26989

Closed

Conversation

@mcollina
Copy link
Member

commented Mar 29, 2019

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 mcollina requested review from jasnell and nodejs/tsc Mar 29, 2019

@nodejs-github-bot

This comment has been minimized.

Copy link

commented Mar 29, 2019

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

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

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

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

@targos

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

why semver-major?

@mcollina

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

why semver-major?

I don't know really.

@mcollina

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

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

I'm happy to include that in this PR.

@mcollina mcollina added semver-minor and removed semver-major labels Mar 29, 2019

changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/26989
description: AsyncIterator support is not experimental anymore.

This comment has been minimized.

Copy link
@jasnell

jasnell Mar 29, 2019

Member
Suggested change
description: AsyncIterator support is not experimental anymore.
description: AsyncIterator support is no longer experimental.

@BridgeAR BridgeAR added author ready and removed author ready labels Mar 31, 2019

@benjamingr
Copy link
Member

left a comment

🎉

@mcollina mcollina force-pushed the mcollina:make-readable-async-iterator-stable branch from 9b5ab18 to 8df8328 Apr 1, 2019

@mcollina

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

1 similar comment
@nodejs-github-bot

This comment has been minimized.

@mcollina

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

I've also marked readline AsyncIterator support stable.

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

@benjamingr
Copy link
Member

left a comment

Still LGTM

@BridgeAR
Copy link
Member

left a comment

Still LGTM

@nodejs-github-bot

This comment has been minimized.

@mcollina

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2019

1 similar comment
@nodejs-github-bot

This comment has been minimized.

@mcollina

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2019

Landed in 5b8434e...4de5e0a

@mcollina mcollina closed this Apr 3, 2019

@mcollina mcollina deleted the mcollina:make-readable-async-iterator-stable branch Apr 3, 2019

mcollina added a commit that referenced this pull request Apr 3, 2019

stream: make Symbol.asyncIterator support stable
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>

mcollina added a commit that referenced this pull request Apr 3, 2019

readline: make Symbol.asyncIterator support stable
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 4, 2019

stream: make Symbol.asyncIterator support stable
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 4, 2019

readline: make Symbol.asyncIterator support stable
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 9, 2019

stream: make Symbol.asyncIterator support stable
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 9, 2019

readline: make Symbol.asyncIterator support stable
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 BethGriggs referenced this pull request Apr 9, 2019

Merged

v11.14.0 proposal #27163

BethGriggs added a commit that referenced this pull request Apr 10, 2019

stream: make Symbol.asyncIterator support stable
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

readline: make Symbol.asyncIterator support stable
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)
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)
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)
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.