Skip to content

stream: group all properties using defineProperties - #31144

Closed
antsmartian wants to merge 1 commit into
nodejs:masterfrom
antsmartian:refactor_stream
Closed

stream: group all properties using defineProperties#31144
antsmartian wants to merge 1 commit into
nodejs:masterfrom
antsmartian:refactor_stream

Conversation

@antsmartian

@antsmartian antsmartian commented Dec 31, 2019

Copy link
Copy Markdown
Contributor

Simple refactor. Moved all the property definition into a single block and removed duplicated comments across the file.

@nodejs-github-bot nodejs-github-bot added the stream Issues and PRs related to Node.js streams. label Dec 31, 2019
@nodejs-github-bot

nodejs-github-bot commented Dec 31, 2019

Copy link
Copy Markdown
Collaborator

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

@Trott

Trott commented Dec 31, 2019

Copy link
Copy Markdown
Member

@nodejs/streams

mcollina
mcollina approved these changes Jan 1, 2020
View reviewed changes

@mcollina mcollina 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.

lgtm

@BridgeAR BridgeAR added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Jan 1, 2020
@nodejs-github-bot

nodejs-github-bot commented Jan 1, 2020

Copy link
Copy Markdown
Collaborator

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

ronag
ronag approved these changes Jan 1, 2020
View reviewed changes

@ronag ronag 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.

Nice

BridgeAR pushed a commit that referenced this pull request Jan 2, 2020
@antsmartian @BridgeAR
stream: group all properties using defineProperties
2257e80 
PR-URL: #31144
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR

BridgeAR commented Jan 2, 2020

Copy link
Copy Markdown
Member

Landed in 2257e80 🎉

@BridgeAR BridgeAR closed this Jan 2, 2020
BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
@antsmartian @BridgeAR
stream: group all properties using defineProperties
5bf2772 
PR-URL: #31144
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ronag

ronag commented Jan 4, 2020

Copy link
Copy Markdown
Member

@antsmartian: Would you mind doing the same for Writable and Duplex?

@antsmartian antsmartian mentioned this pull request Jan 5, 2020
Closed
4 tasks
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
Merged
targos pushed a commit that referenced this pull request Jan 14, 2020
@antsmartian @targos
stream: group all properties using defineProperties
504e6d4 
PR-URL: #31144
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jan 14, 2020
@antsmartian @targos
stream: group all properties using defineProperties
5edfd50 
PR-URL: #31144
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Jan 15, 2020
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
@antsmartian @BethGriggs
stream: group all properties using defineProperties
aad2578 
PR-URL: #31144
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
Merged
addaleax pushed a commit that referenced this pull request Feb 10, 2020
@antsmartian @addaleax
stream: combine properties using defineProperties
0ac04ec 
PR-URL: #31187
Refs: #31144
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@ronag ronag mentioned this pull request Mar 9, 2020
Closed
3 of 4 tasks
ronag pushed a commit to nxtedition/node that referenced this pull request Mar 9, 2020
@antsmartian @ronag
stream: combine properties using defineProperties
abd0fbb 
PR-URL: nodejs#31187
Refs: nodejs#31144
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-PR-URL: nodejs#32164
MylesBorins pushed a commit that referenced this pull request Mar 10, 2020
@antsmartian @MylesBorins
stream: combine properties using defineProperties
90a4d43 
Backport-PR-URL: #32164
PR-URL: #31187
Refs: #31144
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Apr 20, 2020
@antsmartian @targos
stream: combine properties using defineProperties
8d1eeb1 
Backport-PR-URL: #32164
PR-URL: #31187
Refs: #31144
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
lundibundi added a commit to lundibundi/node that referenced this pull request Aug 23, 2020
@lundibundi
stream: fix Readable stream state properties
caeb4e8 
Looks like they have been accidentally moved in
nodejs#31144. This also adds the proxy
properties to Readable since they have been present all this time
and removing them would be breaking.
@lundibundi lundibundi mentioned this pull request Aug 23, 2020
Closed
4 tasks done
lundibundi added a commit to lundibundi/node that referenced this pull request Aug 23, 2020
@lundibundi
stream: fix Readable stream state properties
9cc6c1f 
Looks like they have been accidentally moved in
nodejs#31144.
nodejs-github-bot pushed a commit that referenced this pull request Sep 1, 2020
@lundibundi @nodejs-github-bot
stream: fix Readable stream state properties
e2ffa45 
Looks like they have been accidentally moved in
#31144.

PR-URL: #34886
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
richardlau pushed a commit that referenced this pull request Sep 2, 2020
@lundibundi @richardlau
stream: fix Readable stream state properties
098711e 
Looks like they have been accidentally moved in
#31144.

PR-URL: #34886
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
richardlau pushed a commit that referenced this pull request Sep 3, 2020
@lundibundi @richardlau
stream: fix Readable stream state properties
5f24cea 
Looks like they have been accidentally moved in
#31144.

PR-URL: #34886
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@lundibundi @joesepi
stream: fix Readable stream state properties
eeebc31 
Looks like they have been accidentally moved in
nodejs#31144.

PR-URL: nodejs#34886
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina