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.

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

lib,tools: use internal/url, restrict url in internal code #49590

Closed
wants to merge 2 commits into from
Closed

lib,tools: use internal/url, restrict url in internal code #49590

wants to merge 2 commits into from

Conversation

LiviaMedeiros
Copy link
Contributor

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup
  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added debugger Issues and PRs related to the debugger subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Sep 10, 2023
@LiviaMedeiros LiviaMedeiros force-pushed the lib-eliminate-public-url branch 2 times, most recently from b642b02 to cedd422 Compare September 10, 2023 15:00
@LiviaMedeiros LiviaMedeiros added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Sep 10, 2023
@LiviaMedeiros LiviaMedeiros changed the title lib: restrict internal code from using public url module lib,tools: use internal/url, restrict url in internal code Sep 10, 2023
@LiviaMedeiros LiviaMedeiros removed the test_runner Issues and PRs related to the test runner subsystem. label Sep 10, 2023
@LiviaMedeiros LiviaMedeiros added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 10, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 10, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Sep 11, 2023
Open
18 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@LiviaMedeiros LiviaMedeiros added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 11, 2023
aduh95
aduh95 reviewed Sep 11, 2023
View reviewed changes
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

I think we want to keep node:url in the bootstrap path so it ends up in the snapsot

lib/internal/bootstrap/switches/is_main_thread.js Outdated Show resolved Hide resolved
test/parallel/test-bootstrap-modules.js Outdated Show resolved Hide resolved
@LiviaMedeiros LiviaMedeiros removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 11, 2023
@LiviaMedeiros LiviaMedeiros added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 11, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 11, 2023
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@LiviaMedeiros LiviaMedeiros added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 11, 2023
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Sep 12, 2023
Open
19 tasks
@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 13, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 13, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in ccf46ba...f5bb2c7

nodejs-github-bot pushed a commit that referenced this pull request Sep 13, 2023
@LiviaMedeiros @nodejs-github-bot
debugger: use internal/url.URL instead of url.parse
2a0e5de 
PR-URL: #49590
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 13, 2023
@LiviaMedeiros @nodejs-github-bot
tools: restrict internal code from using public url module
f5bb2c7 
PR-URL: #49590
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@LiviaMedeiros LiviaMedeiros mentioned this pull request Sep 14, 2023
Closed
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@LiviaMedeiros @ruyadorno
debugger: use internal/url.URL instead of url.parse
6fcb51d 
PR-URL: #49590
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@LiviaMedeiros @ruyadorno
tools: restrict internal code from using public url module
08ffc63 
PR-URL: #49590
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
This was referenced Sep 28, 2023
Closed
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@LiviaMedeiros @alexfernandez
debugger: use internal/url.URL instead of url.parse
fd24549 
PR-URL: nodejs#49590
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@LiviaMedeiros @alexfernandez
tools: restrict internal code from using public url module
c3ee079 
PR-URL: nodejs#49590
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljharb ljharb ljharb approved these changes

@atlowChemi atlowChemi atlowChemi approved these changes

@aduh95 aduh95 aduh95 approved these changes

@targos targos targos approved these changes

@anonrig anonrig anonrig approved these changes

@lpinca lpinca lpinca approved these changes

@GeoffreyBooth GeoffreyBooth GeoffreyBooth 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. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. debugger Issues and PRs related to the debugger subsystem. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@LiviaMedeiros @nodejs-github-bot @ljharb @GeoffreyBooth @lpinca @anonrig @targos @aduh95 @atlowChemi