Skip to content

doc: remove Legacy status from querystring - #44912

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
Trott:querystring
Oct 9, 2022
Merged

doc: remove Legacy status from querystring#44912
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
Trott:querystring

Conversation

@Trott

@TrottTrott commented Oct 7, 2022

Copy link
Copy Markdown
Member

Closes: #44911

@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. querystring Issues and PRs related to the built-in querystring module. labels Oct 7, 2022
@TrottTrott mentioned this pull request Oct 7, 2022
Closed
mcollina
mcollina approved these changes Oct 7, 2022
View reviewed changes

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@targos

targos commented Oct 7, 2022•
edited
Loading

Copy link
Copy Markdown
Member

I'm not able to check who moved that API to Legacy but we should probably ping them.

@anonrig

anonrig commented Oct 7, 2022

Copy link
Copy Markdown
Member

I'm in favor of removing legacy and improving querystring module performance.

@Trott

Trott commented Oct 7, 2022

Copy link
Copy Markdown
MemberAuthor

I'm not able to check who moved that API to Legacy but we should probably ping them.

It was @jasnell in 32ade6997ba.

@jasnell

jasnell commented Oct 8, 2022

Copy link
Copy Markdown
Member

I'm -0 on this. I don't see much justification. Legacy status doesn't mean we can't take improvements at all. Won't block tho.

@targos

targos commented Oct 8, 2022

Copy link
Copy Markdown
Member

@jasnell see #44911. The problem is that many people confuse Legacy and Deprecated.

@kibertoad

kibertoad commented Oct 8, 2022

Copy link
Copy Markdown
Contributor

@jasnell if it doesn't mean it can't be improved, and it doesn't mean there are strong reasons not to use it in the new code, then what does it even mean?

@jasnell

jasnell commented Oct 8, 2022

Copy link
Copy Markdown
Member

@targos yeah that's why I'm not blocking :-)

@kibertoad ... It means there is a standard API alternative available that is preferred, which for me (at least) counts as a strong reason not to use it.

@kibertoad

kibertoad commented Oct 8, 2022

Copy link
Copy Markdown
Contributor

@jasnell but can it be called a clearly preferred alternative? it is both slower and handles significantly less cases. it's basically a glorified toString

@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 9, 2022
@jasnell

jasnell commented Oct 9, 2022

Copy link
Copy Markdown
Member

@kibertoad ... We are all welcomed to our own opinions. I did clearly state that mine was non-blocking. I think legacy status is better here and legacy doesn't mean that continued improvement cannot be made.

@TrottTrott added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 9, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 9, 2022
@nodejs-github-bot
nodejs-github-bot merged commit c163bf9 into nodejs:mainOct 9, 2022
@nodejs-github-bot

nodejs-github-bot commented Oct 9, 2022

Copy link
Copy Markdown
Collaborator

Landed in c163bf9

@Trott
Trott deleted the querystring branch October 9, 2022 16:05
danielleadams pushed a commit that referenced this pull request Oct 11, 2022
@Trott@danielleadams
doc: remove Legacy status from querystring
842bc64
Closes: #44911
PR-URL: #44912Fixes: #44911
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@danielleadamsdanielleadams mentioned this pull request Oct 11, 2022
Merged
@ChALkeRChALkeR mentioned this pull request Apr 19, 2026
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@lpincalpincalpinca approved these changes
@targostargostargos approved these changes
@gireeshpunathilgireeshpunathilgireeshpunathil approved these changes
@ErickWendelErickWendelErickWendel approved these changes
@trivikrtrivikrtrivikr approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.docIssues and PRs related to the documentations.querystringIssues and PRs related to the built-in querystring module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove legacy status from querystring

11 participants

@Trott@targos@anonrig@jasnell@kibertoad@nodejs-github-bot@mcollina@lpinca@gireeshpunathil@ErickWendel@trivikr