-
Notifications
You must be signed in to change notification settings - Fork 525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: try to reduce amount of http2 specific logic #2816
Conversation
@metcoder95 can you help me figure out why this change breaks tests? |
Sure thing, I'll take a look; sorry for being quite slow has been busy lately 🙇 |
@metcoder95 some more changes, I think you get the gist |
c0d2eae
to
45803d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far, the idea LGTM; I would like to see if we can scope the H2 logic even more, but we can do it in another PR, I'll check the test first 👍
util.destroy(this, err) | ||
util.destroy(socket, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
util.destroy(this, err) | |
util.destroy(socket, err) | |
this.destroy(err) | |
socket.destroy(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
etc...
Steps towards more clean separation between h1 and h2. Refs: #2816
do you want me to wait for #2839 to land? |
Sorry for that one. Just got an idea and went with it. Hopefully didn't cause you unnecessary work. |
This relates to...
Rationale
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status