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

lib: merge onread handlers for http2 streams & net.Socket by addaleax · Pull Request #22449 · nodejs/node · GitHub

/ node Public

lib: merge onread handlers for http2 streams & net.Socket - #22449

Closed
addaleax wants to merge 2 commits into
nodejs:masterfrom
addaleax:merge-net-socket-onread
ConversationCommits (2)ChecksFiles changed
Closed

lib: merge onread handlers for http2 streams & net.Socket#22449
addaleax wants to merge 2 commits into
nodejs:masterfrom
addaleax:merge-net-socket-onread

Conversation

addaleax commented Aug 21, 2018

Copy link
Copy Markdown
Member

This is a rebased & modified version of @aks-’s #20993, with tests passing locally

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

nodejs-github-bot commented Aug 21, 2018

Copy link
Copy Markdown
Collaborator

@addaleax build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/621/pipeline

nodejs-github-bot added dont-land-on-v6.x http2 Issues or PRs related to the http2 subsystem. net Issues and PRs related to the net subsystem. labels Aug 21, 2018

addaleax commented Aug 21, 2018

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/16652/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1518/
HTTP/2 Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/226/
Net Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/227/

addaleax mentioned this pull request Aug 21, 2018
Closed
4 tasks
cjihrig approved these changes Aug 22, 2018
View reviewed changes
Comment thread lib/internal/stream_base_commons.js Outdated

cjihrig Aug 22, 2018

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

Should this be internalBinding() now?

jasnell Aug 22, 2018

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

Yes.

addaleax force-pushed the merge-net-socket-onread branch from c91fe6e to 737b0fb Compare August 27, 2018 22:02

addaleax commented Aug 27, 2018

Copy link
Copy Markdown
Member Author

Addressed the review comment

CI: https://ci.nodejs.org/job/node-test-pull-request/16803/
HTTP/2 benchmark CI with more iterations: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/230/ (scheduled)

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 27, 2018

addaleax commented Aug 31, 2018

Copy link
Copy Markdown
Member Author

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

addaleax commented Sep 2, 2018

Copy link
Copy Markdown
Member Author

Landed in 1126dfe

addaleax closed this Sep 2, 2018
addaleax deleted the merge-net-socket-onread branch September 2, 2018 11:45
addaleax pushed a commit that referenced this pull request Sep 2, 2018
lib: merge onread handlers for http2 streams & net.Socket
1126dfe 
Refs: #20993
Co-authored-by: Anna Henningsen <anna@addaleax.net>

PR-URL: #22449
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Sep 2, 2018
lib: merge onread handlers for http2 streams & net.Socket
45a6108 
Refs: #20993
Co-authored-by: Anna Henningsen <anna@addaleax.net>

PR-URL: #22449
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Sep 3, 2018
lib: merge onread handlers for http2 streams & net.Socket
f0be053 
Refs: #20993
Co-authored-by: Anna Henningsen <anna@addaleax.net>

PR-URL: #22449
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos mentioned this pull request Sep 5, 2018
Merged
targos pushed a commit that referenced this pull request Sep 6, 2018
lib: merge onread handlers for http2 streams & net.Socket
2a88731 
Refs: #20993
Co-authored-by: Anna Henningsen <anna@addaleax.net>

PR-URL: #22449
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was referenced Sep 6, 2018
Closed
Closed
kjin mentioned this pull request Oct 3, 2018
Closed
4 tasks
addaleax mentioned this pull request Oct 20, 2018
Closed
2 tasks
imcotton mentioned this pull request May 1, 2019
Closed
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
cjihrig cjihrig approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. http2 Issues or PRs related to the http2 subsystem. 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.

5 participants

Back | FazBrowse Home | New Git URL