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

url: add pending-deprecation to url.parse() #47203

Merged
merged 2 commits into from
Mar 23, 2023

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Mar 21, 2023

I propose adding support for --pending-deprecation for the url.parse() method. The new improvements in WHATWG URL and Ada hints us towards a runtime deprecation in the upcoming years.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/url

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. url Issues and PRs related to the legacy built-in url module. labels Mar 21, 2023
@anonrig anonrig force-pushed the pending-deprecation-url-parse branch from 30891b9 to 6ef70eb Compare March 21, 2023 19:40
@anonrig
Copy link
Member Author

anonrig commented Mar 21, 2023

@nodejs/tsc please review

@anonrig anonrig force-pushed the pending-deprecation-url-parse branch from 6ef70eb to e921846 Compare March 21, 2023 19:47
@anonrig anonrig added semver-major PRs that contain breaking changes and should be released in the next major version. deprecations Issues and PRs related to deprecations. labels Mar 21, 2023
@anonrig anonrig force-pushed the pending-deprecation-url-parse branch from e921846 to 476c1ee Compare March 21, 2023 22:10
lib/url.js Outdated Show resolved Hide resolved
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM with @mscdex's suggestion applied.

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

@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Mar 22, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 22, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 23, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 23, 2023
@nodejs-github-bot nodejs-github-bot merged commit 27335cd into nodejs:main Mar 23, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 27335cd

@anonrig anonrig deleted the pending-deprecation-url-parse branch March 23, 2023 19:52
@tniessen
Copy link
Member

Why is this semver-major? IIRC we don't treat "pending deprecation" as breaking (or notable). The deprecation is still documentation-only. The original introduction of the documentation-only status was a notable change, yet not semver-major.

@anonrig anonrig removed the semver-major PRs that contain breaking changes and should be released in the next major version. label Mar 23, 2023
@anonrig
Copy link
Member Author

anonrig commented Mar 23, 2023

Thank you @tniessen. I wasn't aware of that. I removed the label.

@tniessen
Copy link
Member

That way we can backport it :)

RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
PR-URL: #47203
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
PR-URL: #47203
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
PR-URL: #47203
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
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. deprecations Issues and PRs related to deprecations. needs-ci PRs that need a full CI run. url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants