X Tutup
The Wayback Machine - https://web.archive.org/web/20220321210515/https://github.com/nodejs/node/pull/41652
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: iterator helpers synchronous errors #41652

Merged

Conversation

iMoses
Copy link
Contributor

@iMoses iMoses commented Jan 22, 2022

streams/operators/map is no longer a generator function,
instead it returns a called generator so that validation can be
synchronous and not wait for the first iteration

Fixes: #41648

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 22, 2022

Review requested:

test/parallel/test-stream-map.js Outdated Show resolved Hide resolved
Copy link
Member

@benjamingr benjamingr left a comment

Thanks, this looks good to me, it makes sense to also fix this for filter as well :)

Copy link
Member

@mcollina mcollina left a comment

lgtm

ronag
ronag approved these changes Jan 23, 2022
@iMoses iMoses force-pushed the iterator-helpers-sync-validation branch 3 times, most recently from f90e4d9 to 8bdf558 Compare Jan 25, 2022
@benjamingr
Copy link
Member

@benjamingr benjamingr commented Jan 25, 2022

Let's get CI to pass and then land this :)

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 25, 2022

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 25, 2022

Commit Queue failed
- Loading data for nodejs/node/pull/41652
✔  Done loading data for nodejs/node/pull/41652
----------------------------------- PR info ------------------------------------
Title      stream: iterator helpers synchronous errors (#41652)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     iMoses:iterator-helpers-sync-validation -> nodejs:master
Labels     stream, needs-ci
Commits    1
 - stream: iterator helpers synchronous errors
Committers 1
 - iMoses 
PR-URL: https://github.com/nodejs/node/pull/41652
Fixes: https://github.com/nodejs/node/issues/41648
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Robert Nagy 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41652
Fixes: https://github.com/nodejs/node/issues/41648
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Robert Nagy 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - stream: iterator helpers synchronous errors
   ℹ  This PR was created on Sat, 22 Jan 2022 20:36:23 GMT
   ✔  Approvals: 3
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/41652#pullrequestreview-860239785
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41652#pullrequestreview-860241270
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/41652#pullrequestreview-860290302
   ✖  GitHub CI is still running
   ℹ  Last Full PR CI on 2022-01-25T09:58:30Z: https://ci.nodejs.org/job/node-test-pull-request/42132/
- Querying data for job/node-test-pull-request/42132/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/1744970014

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 25, 2022

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 25, 2022

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 25, 2022

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 25, 2022

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 25, 2022

Commit Queue failed
- Loading data for nodejs/node/pull/41652
✔  Done loading data for nodejs/node/pull/41652
----------------------------------- PR info ------------------------------------
Title      stream: iterator helpers synchronous errors (#41652)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     iMoses:iterator-helpers-sync-validation -> nodejs:master
Labels     stream, needs-ci
Commits    1
 - stream: iterator helpers synchronous errors
Committers 1
 - iMoses 
PR-URL: https://github.com/nodejs/node/pull/41652
Fixes: https://github.com/nodejs/node/issues/41648
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Robert Nagy 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41652
Fixes: https://github.com/nodejs/node/issues/41648
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Robert Nagy 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - stream: iterator helpers synchronous errors
   ℹ  This PR was created on Sat, 22 Jan 2022 20:36:23 GMT
   ✔  Approvals: 3
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/41652#pullrequestreview-860239785
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41652#pullrequestreview-860241270
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/41652#pullrequestreview-860290302
   ✖  Last GitHub CI failed
   ℹ  Last Full PR CI on 2022-01-25T14:23:50Z: https://ci.nodejs.org/job/node-test-pull-request/42140/
- Querying data for job/node-test-pull-request/42140/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/1745928514

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 25, 2022

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 26, 2022

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 26, 2022

 is no longer a generator function,
instead it returns a called generator so that validation can be
synchronous and not wait for the first iteration

Fixes: nodejs#41648
@iMoses iMoses force-pushed the iterator-helpers-sync-validation branch from 1947faf to 3fe2810 Compare Jan 27, 2022
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 27, 2022

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 27, 2022

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 27, 2022

@nodejs-github-bot nodejs-github-bot merged commit 5d9caa1 into nodejs:master Jan 27, 2022
53 checks passed
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jan 27, 2022

Landed in 5d9caa1

@benjamingr
Copy link
Member

@benjamingr benjamingr commented Jan 27, 2022

Congrats on your first contribution and patience @iMoses - let's talk about the next one :)?

Linkgoron added a commit to Linkgoron/node that referenced this issue Jan 31, 2022
 is no longer a generator function,
instead it returns a called generator so that validation can be
synchronous and not wait for the first iteration

Fixes: nodejs#41648

PR-URL: nodejs#41652
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
ruyadorno added a commit that referenced this issue Feb 8, 2022
 is no longer a generator function,
instead it returns a called generator so that validation can be
synchronous and not wait for the first iteration

Fixes: #41648

PR-URL: #41652
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants
X Tutup