-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
url: use existing handlers instead of duplicated code #19267
Conversation
/сс @nodejs/collaborators |
@nodejs/url |
The update looks good to me. I'd actually like to keep the parse methods as current them because it's easier to compare with the spec if they are paired, and it's also good considering maintainability. However, as the benchmark result is getting better, let me say ±0 on this. |
@nodejs/collaborators Hello! Run CI for this PR, please |
CI failures appear to be unrelated. |
Landed in a892d9a 🎉 |
PR-URL: nodejs#19267 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #19267 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesBenchmark results: