Skip to content
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

src: cache urlpattern properties #57465

Closed

Conversation

JonasBa
Copy link
Contributor

@JonasBa JonasBa commented Mar 14, 2025

Cache urlpattern instance properties.

This PR is stacked on top of #57452, and will be converted to ready for review once but the base lands

@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
@JonasBa JonasBa force-pushed the jb/urlpattern/cache-urlpattern-values branch from dd09c80 to 92bd6b0 Compare March 14, 2025 17:38
@JonasBa JonasBa force-pushed the jb/urlpattern/cache-urlpattern-values branch 2 times, most recently from f049360 to c43db3e Compare March 14, 2025 23:48
@anonrig
Copy link
Member

anonrig commented Mar 17, 2025

@JonasBa can you rebase?

@JonasBa JonasBa force-pushed the jb/urlpattern/cache-urlpattern-values branch from c43db3e to 6b48b08 Compare March 17, 2025 23:05
@JonasBa JonasBa marked this pull request as ready for review March 17, 2025 23:05
Copy link

codecov bot commented Mar 18, 2025

Codecov Report

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

Project coverage is 90.23%. Comparing base (38390e5) to head (6b48b08).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
src/node_url_pattern.cc 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #57465   +/-   ##
=======================================
  Coverage   90.22%   90.23%           
=======================================
  Files         629      629           
  Lines      184845   184846    +1     
  Branches    36205    36212    +7     
=======================================
+ Hits       166784   166790    +6     
+ Misses      11016    11014    -2     
+ Partials     7045     7042    -3     
Files with missing lines Coverage Δ
src/node_url_pattern.h 0.00% <ø> (ø)
src/node_url_pattern.cc 79.06% <0.00%> (-2.42%) ⬇️

... and 25 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

@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 18, 2025
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 19, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/57465
✔  Done loading data for nodejs/node/pull/57465
----------------------------------- PR info ------------------------------------
Title      src: cache urlpattern properties (#57465)
Author     Jonas <jonas@badalic.com> (@JonasBa)
Branch     JonasBa:jb/urlpattern/cache-urlpattern-values -> nodejs:main
Labels     c++, whatwg-url, author ready, needs-ci
Commits    2
 - src: cache urlpattern properties
 - fixup! src: cache urlpattern properties
Committers 1
 - JonasBa <jonas@badalic.com>
PR-URL: https://github.com/nodejs/node/pull/57465
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/57465
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 14 Mar 2025 17:38:04 GMT
   ✔  Approvals: 3
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/57465#pullrequestreview-2694662353
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/57465#pullrequestreview-2694673339
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/57465#pullrequestreview-2699142915
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2025-03-18T13:55:37Z: https://ci.nodejs.org/job/node-test-pull-request/65823/
- Querying data for job/node-test-pull-request/65823/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/13951301572

anonrig pushed a commit that referenced this pull request Mar 19, 2025
PR-URL: #57465
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@anonrig
Copy link
Member

anonrig commented Mar 19, 2025

Landed in 015cd20

@anonrig anonrig closed this Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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-failed An error occurred while landing this pull request using GitHub Actions. needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants