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

stream: use for...of by trivikr · Pull Request #30960 · nodejs/node · GitHub

/ node Public

stream: use for...of - #30960

Closed
trivikr wants to merge 3 commits into
nodejs:masterfrom
trivikr:use-for-of-stream
ConversationCommits (3)ChecksFiles changed
Closed

stream: use for...of#30960
trivikr wants to merge 3 commits into
nodejs:masterfrom
trivikr:use-for-of-stream

Conversation

trivikr commented Dec 14, 2019

Copy link
Copy Markdown
Member

Searched for regular expression for \([^\.]*.length in lib and made changes in _stream_* files

Refs: #30910 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

nodejs-github-bot added the stream Issues and PRs related to the stream subsystem. label Dec 14, 2019

richardlau commented Dec 14, 2019

Copy link
Copy Markdown
Member

I think these files are used in https://github.com/nodejs/readable-stream and still support older versions of Node.js so the performance would need to be checked on those?

cc @nodejs/streams

mcollina approved these changes Dec 14, 2019
View reviewed changes

mcollina left a comment •
edited
Loading

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

LGTM as long as they do not land on 10 (that's where readable-stream is currently cut from)

mcollina commented Dec 14, 2019

Copy link
Copy Markdown
Member

Those do not sit in a hot path, as far as I know.

trivikr mentioned this pull request Dec 14, 2019
Closed
3 tasks
trivikr added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 14, 2019

nodejs-github-bot commented Dec 14, 2019

Copy link
Copy Markdown
Collaborator

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

soar-beep added a commit to soar-beep/node that referenced this pull request Dec 16, 2019
assert: use for...of
56a8b7d 
Refs: nodejs#30960
soar-beep mentioned this pull request Dec 16, 2019
Closed
2 tasks
trivikr added a commit that referenced this pull request Dec 16, 2019
stream: use for...of
ce49f90 
PR-URL: #30960
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

trivikr commented Dec 16, 2019

Copy link
Copy Markdown
Member Author

Landed in ce49f90

trivikr closed this Dec 16, 2019
trivikr deleted the use-for-of-stream branch December 16, 2019 23:21
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
stream: use for...of
5ca29d8 
PR-URL: #30960
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins mentioned this pull request Dec 17, 2019
Merged
Trott pushed a commit to Trott/io.js that referenced this pull request Dec 18, 2019
assert: use for...of
145116b 
Refs: nodejs#30960

PR-URL: nodejs#30983
Refs: nodejs#30960
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
trivikr mentioned this pull request Dec 24, 2019
Closed
2 tasks
BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
assert: use for...of
f8aa365 
Refs: #30960

PR-URL: #30983
Refs: #30960
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Jan 14, 2020
stream: use for...of
f6acf9a 
PR-URL: #30960
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jan 14, 2020
assert: use for...of
9da5444 
Refs: #30960

PR-URL: #30983
Refs: #30960
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos mentioned this pull request Jan 15, 2020
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
stream: use for...of
3872a02 
PR-URL: #30960
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
assert: use for...of
6909e3e 
Refs: #30960

PR-URL: #30983
Refs: #30960
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins mentioned this pull request Feb 8, 2020
Merged
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

mcollina mcollina approved these changes
jasnell jasnell approved these changes
Trott Trott approved these changes
addaleax addaleax approved these changes
lpinca lpinca approved these changes
BridgeAR BridgeAR approved these changes
+1 more reviewer
targos targos 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. 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.

10 participants

Back | FazBrowse Home | New Git URL