Skip to content

src: define urlpattern components via macro #57452

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

src: define urlpattern components via macro #57452

wants to merge 2 commits into from
+52 −187

Conversation

JonasBa
Copy link
Contributor

@JonasBa JonasBa commented Mar 14, 2025

Remove the tedious definitions in favor of a macro

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Mar 14, 2025
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 14, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 14, 2025
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/65733/

@anonrig anonrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 14, 2025
@codecov Codecov
Copy link

codecov bot commented Mar 14, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.21%. Comparing base (3329efe) to head (c7eb31d).
Report is 216 commits behind head on main.

Files with missing lines Patch % Lines
src/node_url_pattern.cc 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #57452      +/-   ##
==========================================
+ Coverage   90.20%   90.21%   +0.01%     
==========================================
  Files         629      629              
  Lines      184948   184805     -143     
  Branches    36204    36192      -12     
==========================================
- Hits       166837   166730     -107     
+ Misses      11057    11038      -19     
+ Partials     7054     7037      -17
Files with missing lines Coverage Δ
src/node_url_pattern.h 0.00% <ø> (ø)
src/node_url_pattern.cc 79.80% <75.00%> (-2.17%) ⬇️

... and 51 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/65740/

@JonasBa
Copy link
Contributor Author

JonasBa commented Mar 14, 2025

Is there a recommended workflow for stacked pull requests? I have a change that would like to see land after this as well. I can target that PR with this base branch, which will then auto update once this one is merged.

@JonasBa JonasBa force-pushed the jb/urlpattern/cache-result branch from 6c18c25 to 523f8b5 Compare March 14, 2025 11:09
anonrig
anonrig reviewed Mar 14, 2025
View reviewed changes
@JonasBa JonasBa force-pushed the jb/urlpattern/cache-result branch 2 times, most recently from 4a2a7ba to ca83248 Compare March 14, 2025 13:08
@JonasBa JonasBa force-pushed the jb/urlpattern/cache-result branch from ca83248 to c7eb31d Compare March 14, 2025 16:59
@BridgeAR
Copy link
Member

Is there a recommended workflow for stacked pull requests? I have a change that would like to see land after this as well. I can target that PR with this base branch, which will then auto update once this one is merged.

Ideally it is opened after this lands. That's at least how we mostly handled it. I guess this won't take that long to land (at least 2 more days though).

@JonasBa
Copy link
Contributor Author

JonasBa commented Mar 14, 2025

Thank you @BridgeAR, I will wait to open that one then!

@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 14, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 14, 2025
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/65754/

@JonasBa JonasBa mentioned this pull request Mar 14, 2025
Closed
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/65770/

@addaleax addaleax added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Mar 15, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 16, 2025
@nodejs-github-bot
Copy link
Collaborator

Landed in 26c4851...d5ac3e3

nodejs-github-bot pushed a commit that referenced this pull request Mar 16, 2025
@JonasBa @nodejs-github-bot
src: define urlpattern components using a macro
d5ac3e3 
PR-URL: #57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
aduh95 pushed a commit that referenced this pull request Mar 18, 2025
@JonasBa @aduh95
src: define urlpattern components using a macro
6ee15c6 
PR-URL: #57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@github-actions github-actions bot mentioned this pull request Mar 30, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 1, 2025
@JonasBa @RafaelGSS
src: define urlpattern components using a macro
e8f26ed 
PR-URL: #57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
RafaelGSS pushed a commit that referenced this pull request Apr 1, 2025
@JonasBa @RafaelGSS
src: define urlpattern components using a macro
8b8bf83 
PR-URL: #57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@RafaelGSS RafaelGSS mentioned this pull request Apr 8, 2025
Open
RafaelGSS pushed a commit to RafaelGSS/node that referenced this pull request Apr 8, 2025
@JonasBa @RafaelGSS
src: define urlpattern components using a macro
4aa03c8 
PR-URL: nodejs#57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@aduh95 aduh95 mentioned this pull request Apr 11, 2025
Open
RafaelGSS pushed a commit that referenced this pull request Apr 14, 2025
@JonasBa @RafaelGSS
src: define urlpattern components using a macro
7112abe 
PR-URL: #57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
RafaelGSS pushed a commit that referenced this pull request Apr 14, 2025
@JonasBa @RafaelGSS
src: define urlpattern components using a macro
4499de9 
PR-URL: #57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
aduh95 pushed a commit that referenced this pull request Apr 14, 2025
@JonasBa @aduh95
src: define urlpattern components using a macro
70e3500 
PR-URL: #57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
aduh95 pushed a commit that referenced this pull request Apr 14, 2025
@JonasBa @aduh95
src: define urlpattern components using a macro
1e82948 
PR-URL: #57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
aduh95 pushed a commit that referenced this pull request Apr 15, 2025
@JonasBa @aduh95
src: define urlpattern components using a macro
c8f0ea5 
PR-URL: #57452
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@RafaelGSS RafaelGSS added dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. labels Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anonrig anonrig anonrig approved these changes

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@aduh95 aduh95 aduh95 approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@JonasBa @nodejs-github-bot @BridgeAR @jasnell @addaleax @anonrig @aduh95 @RaisinTen @RafaelGSS