-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
http2: remove support for priority signaling #58293
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
Conversation
Review requested:
|
cc @nodejs/tsc |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58293 +/- ##
=======================================
Coverage 90.21% 90.22%
=======================================
Files 635 635
Lines 187580 187565 -15
Branches 36853 36848 -5
=======================================
+ Hits 169231 169236 +5
+ Misses 11108 11102 -6
+ Partials 7241 7227 -14
🚀 New features to boost your workflow:
|
There is a typo in the second commit title: priotity -> priority. |
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.
LGTM
Blocked on #58313 |
I realized there were a few missing mentions in the docs, opened #58504 which should land before this one. |
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.
LGTM. Let's start a CITGM before landing this on v24.x.
I've removed the nghttp2 update from this PR since it won't be semver-major once this has landed. The first commit is from #58504, to be removed from this PR once the other PR has landed on |
CITGM(
|
Signed-off-by: Matteo Collina <[email protected]> Co-authored-by: Antoine du Hamel <[email protected]> Refs: https://datatracker.ietf.org/doc/html/rfc9113#section-5.3.1
Landed in a631264 |
This supersedes #57269 as it also patches some failing test that caught a breaking change in nghttp2.
Technically, this is a breaking change for us, but I ask the @nodejs/tsc to land it in v24.x, as it would massively simplify long-term maintenance of the v24.x line.