Repository navigation
quic/stream: create gen. getWriter for stream/iter - #66513
martenrichter wants to merge 3 commits into
Conversation
|
Review requested:
|
|
@jasnell @pimterry |
|
P.S.: There seem to be some bugs remaining. (I thought I had run all tests, but it seems that I only ran lint and not a full build.) But discussing the structure should work anyway. |
2f1920c to
40db343
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #66513 +/- ##
==========================================
+ Coverage 90.45% 90.46% +0.01%
==========================================
Files 791 792 +1
Lines 276806 276915 +109
Branches 53190 53199 +9
==========================================
+ Hits 250373 250503 +130
+ Misses 16842 16826 -16
+ Partials 9591 9586 -5
🚀 New features to boost your workflow:
|
40db343 to
69719cd
Compare
|
Ok, now, everything works according to the local tests. |
|
I do not understand the build failures. It complains about an unknown compiler option? Do I need to rebase? |
|
Looks like yes - it's complaining about That said, you'll then run into a separate issue, because that same V8 update has also independently broken the QUIC build 😆. Fix for that landing shortly, it's here: #66603 |
f49753c to
8a94755
Compare
|
I have moved (I have not rebased, as the fixes for building quic are not on main yet). |
The get writer method of QuicStream is mostly generell enough to be usuable in other contexts. This PR is an attempt to move the code out of the QuicStream object to be usuable in other parts of node.js. Fixes nodejs#66508 Signed-off-by: Marten Richter <marten.richter@freenet.de>
8a94755 to
fe7ad98
Compare
|
|
||
| // TODO(@jasnell) Temporarily ignoring c8 coverage for this file while tests | ||
| // are still being developed. | ||
| /* c8 ignore start */ |
There was a problem hiding this comment.
Moving this to internal/stream/iter likely makes the most sense
3a2403a to
5393b34
Compare
The get writer method of QuicStream is mostly
generell enough to be usuable in other contexts.
This PR is an attempt to move the code out
of the QuicStream object to be usuable in other
parts of node.js.
Fixes #66508