Skip to content

http2: Reuse ._onTimeout() to Http2Session and Http2Stream classes - #33354

Closed
rickyes wants to merge 2 commits into
nodejs:masterfrom
rickyes:reuse-http2-internal-onTimeout
Closed

http2: Reuse ._onTimeout() to Http2Session and Http2Stream classes#33354
rickyes wants to merge 2 commits into
nodejs:masterfrom
rickyes:reuse-http2-internal-onTimeout

Conversation

@rickyes

@rickyesrickyes commented May 11, 2020

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

@nodejs-github-botnodejs-github-bot added the http2 Issues or PRs related to the http2 subsystem. label May 11, 2020
benjamingr
benjamingr reviewed May 11, 2020
View reviewed changes
Comment threadlib/internal/http2/core.js
benjamingr
benjamingr reviewed May 11, 2020
View reviewed changes
Comment threadlib/internal/http2/core.js Outdated
@nodejsnodejs deleted a comment from benjamingrMay 11, 2020
@benjamingr

benjamingr commented May 11, 2020

Copy link
Copy Markdown
Member

@yashLadha hey: you deleted my comment, I'm a bit confused and not not entirely sure how that's possible :]

Would you mind explaining what happened I'm a bit baffled?

@yashLadha

yashLadha commented May 12, 2020

Copy link
Copy Markdown
Contributor

@benjamingr After I commented, i checked out code locally and saw that the method is a class method and after which I deleted my commented. Don't know why your comment got deleted 😕. I was not able to see your comment when I deleted my comment. Probably I haven't refreshed the page. Sorry for any confusion it caused.

@yashLadha

yashLadha commented May 12, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Also I think there is some hook in NodeJS repo because i deleted my comment only 😕

@BridgeAR

BridgeAR commented May 23, 2020

Copy link
Copy Markdown
Member

@benjamingr I guess it's a github bug. I reported it to github to have a closer look.

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 23, 2020
@nodejs-github-bot

nodejs-github-bot commented May 23, 2020

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented May 23, 2020•
edited by BridgeAR
Loading

Copy link
Copy Markdown
Collaborator

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

@BridgeAR

BridgeAR commented May 23, 2020

Copy link
Copy Markdown
Member

Landed in 7a094dc

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 23, 2020
@rickyes@BridgeAR
http2: reuse ._onTimeout() in Http2Session and Http2Stream classes
7a094dc
PR-URL: nodejs#33354
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@rickyes
rickyes deleted the reuse-http2-internal-onTimeout branch May 24, 2020 03:41
codebytere pushed a commit that referenced this pull request Jun 18, 2020
@rickyes@codebytere
http2: reuse ._onTimeout() in Http2Session and Http2Stream classes
8457033
PR-URL: #33354
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@codebyterecodebytere mentioned this pull request Jun 28, 2020
Merged
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@rickyes@codebytere
http2: reuse ._onTimeout() in Http2Session and Http2Stream classes
7338cfd
PR-URL: #33354
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
codebytere pushed a commit that referenced this pull request Jul 8, 2020
@rickyes@codebytere
http2: reuse ._onTimeout() in Http2Session and Http2Stream classes
bcdf4c8
PR-URL: #33354
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@codebyterecodebytere mentioned this pull request Jul 13, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benjamingrbenjamingrbenjamingr approved these changes
@BridgeARBridgeARBridgeAR approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.http2Issues or PRs related to the http2 subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rickyes@benjamingr@yashLadha@BridgeAR@nodejs-github-bot