http: remove pause()/resume() from parser bindings#22076
http: remove pause()/resume() from parser bindings#22076addaleax wants to merge 1 commit intonodejs:masterfrom
pause()/resume() from parser bindings#22076Conversation
As far as I can tell, these are effectively no-ops (`http_parser_pause` only sets the `http_errno` flag, but that isn’t read anywhere), so removing them cleans things up a tiny bit. The corresponding regression test remains intact.
|
They're not a no-op. The |
|
@indutny Thanks, that’s why I pinged the team… is there any behaviour that we could test against? It’s not quite ideal that all tests are passing without this, I’d say. |
|
I'm pretty sure that there's a testable effect of having pause/resume, not sure if it is desirable though 😛 It'd be great indeed if someone would write a test for it. |
|
Why do we have to pause the parser as well? We are already pausing the underlining socket. |
|
Any progress here? |
|
@addaleax Is this land-able after a rebase? Or is there something that has to happen here? (Write a test or something?) |
|
@Trott Yes, this is more of a situation where we should test that these methods actually do something (and if it turns out that they actually don’t, we can still remove them). It’s on my backlog 😄 |
|
FWIW, needs a rebase. |
|
Ping @addaleax |
As far as I can tell, these are effectively no-ops
(
http_parser_pauseonly sets thehttp_errnoflag,but that isn’t read anywhere), so removing them cleans
things up a tiny bit.
The corresponding regression test remains intact.
/cc @nodejs/http-parser
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes