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: move FromNamespacedPath to path.cc #53540

Merged

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Jun 21, 2024

Since ->starts_with() is C++20, this pull-request can not land on v18 and v20.

@anonrig anonrig added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Jun 21, 2024
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/url

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jun 21, 2024
@anonrig anonrig force-pushed the refactor-from-namespaced-path branch from a73098a to ff82bc5 Compare June 21, 2024 22:15
@nodejs-github-bot
Copy link
Collaborator

src/path.h Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

@anonrig
Copy link
Member Author

anonrig commented Jun 26, 2024

cc @nodejs/cpp-reviewers

@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 Jun 26, 2024
@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 Jun 27, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/53540
✔  Done loading data for nodejs/node/pull/53540
----------------------------------- PR info ------------------------------------
Title      src: move `FromNamespacedPath` to path.cc (#53540)
Author     Yagiz Nizipli  (@anonrig)
Branch     anonrig:refactor-from-namespaced-path -> nodejs:main
Labels     c++, lib / src, author ready, needs-ci, dont-land-on-v18.x, dont-land-on-v20.x
Commits    1
 - src: move `FromNamespacedPath` to path.cc
Committers 1
 - Yagiz Nizipli 
PR-URL: https://github.com/nodejs/node/pull/53540
Reviewed-By: Daniel Lemire 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53540
Reviewed-By: Daniel Lemire 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 21 Jun 2024 22:15:46 GMT
   ✔  Approvals: 1
   ✔  - Daniel Lemire (@lemire): https://github.com/nodejs/node/pull/53540#pullrequestreview-2142818588
   ✘  This PR needs to wait 31 more hours to land (or 0 hours if there is one more approval)
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-06-26T13:05:50Z: https://ci.nodejs.org/job/node-test-pull-request/59977/
- Querying data for job/node-test-pull-request/59977/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/9698314507

@anonrig anonrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 27, 2024
Copy link
Contributor

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

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

LGTM 🫡

src/node_url.cc Show resolved Hide resolved
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 27, 2024
@nodejs-github-bot nodejs-github-bot merged commit 3036066 into nodejs:main Jun 27, 2024
60 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 3036066

aduh95 pushed a commit that referenced this pull request Jul 12, 2024
PR-URL: #53540
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
@aduh95 aduh95 mentioned this pull request Jul 12, 2024
aduh95 pushed a commit that referenced this pull request Jul 16, 2024
PR-URL: #53540
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
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++. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants