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

net: remove Socket.prototoype.read by addaleax · Pull Request #18568 · nodejs/node · GitHub

/ node Public

net: remove Socket.prototoype.read - #18568

Closed
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:net-no-consuming
ConversationCommits (1)ChecksFiles changed
Closed

net: remove Socket.prototoype.read#18568
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:net-no-consuming

Conversation

addaleax commented Feb 4, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Unused since 34b535f.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

net

CI: https://ci.nodejs.org/job/node-test-commit/15943/

addaleax added net Issues and PRs related to the net subsystem. lts-watch-v6.x labels Feb 4, 2018
nodejs-github-bot added the net Issues and PRs related to the net subsystem. label Feb 4, 2018

joyeecheung commented Feb 5, 2018

Copy link
Copy Markdown
Member

CITGM against master: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1255/
CITGM against this PR: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1256/
CI: https://ci.nodejs.org/job/node-test-pull-request/12937/

XadillaX approved these changes Feb 5, 2018
View reviewed changes

XadillaX left a comment

Copy link
Copy Markdown
Contributor

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.

BridgeAR added arm Issues and PRs related to the ARM architecture. author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed arm Issues and PRs related to the ARM architecture. labels Feb 6, 2018

apapirovski commented Feb 8, 2018

Copy link
Copy Markdown
Contributor

Landed in ef78a1e

apapirovski closed this Feb 8, 2018
apapirovski pushed a commit that referenced this pull request Feb 8, 2018
net: remove Socket.prototoype.read
ef78a1e 
Unused since 34b535f.

PR-URL: #18568
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
net: remove Socket.prototoype.read
081c6ae 
Unused since 34b535f.

PR-URL: #18568
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
net: remove Socket.prototoype.read
804393b 
Unused since 34b535f.

PR-URL: #18568
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
net: remove Socket.prototoype.read
a5596b1 
Unused since 34b535f.

PR-URL: #18568
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins mentioned this pull request Feb 21, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
net: remove Socket.prototoype.read
6ed4e69 
Unused since 34b535f.

PR-URL: #18568
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

gibfahn commented Apr 13, 2018

Copy link
Copy Markdown
Member

Should this land on v6.x and v8.x? @addaleax I know you added the labels, just wanted to double-check as it seems (at first glance) that there's a possibility of this breaking someone who was expecting that function to be there.

addaleax deleted the net-no-consuming branch April 13, 2018 06:43

addaleax commented Apr 13, 2018

Copy link
Copy Markdown
Member Author

@gibfahn The function is inherited from stream.Readable anyway, this was just a wrapper that we don’t need anymore on these versions

BethGriggs pushed a commit that referenced this pull request Apr 13, 2018
net: remove Socket.prototoype.read
3225601 
Unused since 34b535f.

PR-URL: #18568
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins mentioned this pull request May 2, 2018
Merged
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
net: remove Socket.prototoype.read
d44be40 
Unused since 34b535f.

PR-URL: nodejs#18568
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
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
lpinca lpinca approved these changes
BridgeAR BridgeAR approved these changes
+5 more reviewers
apapirovski apapirovski approved these changes
danbev danbev approved these changes
evanlucas evanlucas approved these changes
XadillaX XadillaX approved these changes
hiroppy hiroppy 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. net Issues and PRs related to the net subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL