FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: deprecate --experimental-policy by RafaelGSS · Pull Request #52602 · nodejs/node · GitHub

/ node Public

doc: deprecate --experimental-policy - #52602

Closed
RafaelGSS wants to merge 1 commit into
nodejs:mainfrom
RafaelGSS:doc-deprecate-policy
ConversationCommits (1)ChecksFiles changed
Closed

doc: deprecate --experimental-policy#52602
RafaelGSS wants to merge 1 commit into
nodejs:mainfrom
RafaelGSS:doc-deprecate-policy

Conversation

RafaelGSS commented Apr 19, 2024

Copy link
Copy Markdown
Member

RafaelGSS added the deprecations Issues and PRs related to deprecations. label Apr 19, 2024
RafaelGSS requested a review from richardlau April 19, 2024 19:30
RafaelGSS marked this pull request as ready for review April 19, 2024 19:30

nodejs-github-bot commented Apr 19, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 19, 2024

aduh95 commented Apr 19, 2024

Copy link
Copy Markdown
Contributor

Deprecated features have stronger semver guarentees than experimental features, which is why we don't deprecate experimental features we want to get rid of. However I kinda like the approach of this PR which is marking the feature as deprecated without adding an entry in deprecation.md. IMO it makes sense to also update the existing warning (updating a warning is not semver major AFAIK) to say we are planing to removing that feature.

richardlau commented Apr 19, 2024

Copy link
Copy Markdown
Member

We have deprecated things that were experimental before, e.g. #12243.
I don't think removal needs to go through a full deprecation cycle but doc-deprecating seems like a reasonable low-effort thing we can do.

Full removal (#52583) touches a lot more files so doc-deprecating first means we don't have to rush that through.

marco-ippolito commented Apr 20, 2024

Copy link
Copy Markdown
Member

I believe for experimental features it's good to change the stability to deprecated and remove it in a next minor, without assigning a deprecation code and going in the full cycle

joyeecheung mentioned this pull request Apr 20, 2024
Merged

joyeecheung commented Apr 20, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Would be cool to do what @aduh95 suggested:

IMO it makes sense to also update the existing warning (updating a warning is not semver major AFAIK) to say we are planing to removing that feature.

Though not necessarily in this PR.

MoLow approved these changes Apr 21, 2024
View reviewed changes

MoLow 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

+1 on updating the warning

RafaelGSS added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Apr 21, 2024
avivkeller added experimental Issues and PRs related to experimental features. policy Issues and PRs related to the policy subsystem. labels Apr 21, 2024
RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 22, 2024
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 Apr 22, 2024

nodejs-github-bot commented Apr 22, 2024

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/52602
✔  Done loading data for nodejs/node/pull/52602
----------------------------------- PR info ------------------------------------
Title      doc: deprecate --experimental-policy (#52602)
Author     Rafael Gonzaga  (@RafaelGSS)
Branch     RafaelGSS:doc-deprecate-policy -> nodejs:main
Labels     doc, experimental, author ready, deprecations, policy
Commits    1
 - doc: deprecate --experimental-policy
Committers 1
 - RafaelGSS 
PR-URL: https://github.com/nodejs/node/pull/52602
Reviewed-By: Marco Ippolito 
Reviewed-By: Joyee Cheung 
Reviewed-By: Moshe Atlow 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/52602
Reviewed-By: Marco Ippolito 
Reviewed-By: Joyee Cheung 
Reviewed-By: Moshe Atlow 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 19 Apr 2024 19:30:13 GMT
   ✔  Approvals: 5
   ✔  - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013052255
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013100547
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013333425
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013389396
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013491033
   ✘  Last GitHub CI failed
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/8784408408

mhdawson approved these changes Apr 22, 2024
View reviewed changes

mhdawson 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

RafaelGSS 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 Apr 22, 2024
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 Apr 22, 2024

nodejs-github-bot commented Apr 22, 2024

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/52602
✔  Done loading data for nodejs/node/pull/52602
----------------------------------- PR info ------------------------------------
Title      doc: deprecate --experimental-policy (#52602)
Author     Rafael Gonzaga  (@RafaelGSS)
Branch     RafaelGSS:doc-deprecate-policy -> nodejs:main
Labels     doc, experimental, author ready, deprecations, policy
Commits    1
 - doc: deprecate --experimental-policy
Committers 1
 - RafaelGSS 
PR-URL: https://github.com/nodejs/node/pull/52602
Reviewed-By: Marco Ippolito 
Reviewed-By: Joyee Cheung 
Reviewed-By: Moshe Atlow 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: James M Snell 
Reviewed-By: Michael Dawson 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/52602
Reviewed-By: Marco Ippolito 
Reviewed-By: Joyee Cheung 
Reviewed-By: Moshe Atlow 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: James M Snell 
Reviewed-By: Michael Dawson 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 19 Apr 2024 19:30:13 GMT
   ✔  Approvals: 6
   ✔  - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013052255
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013100547
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013333425
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013389396
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/52602#pullrequestreview-2013491033
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/52602#pullrequestreview-2014748381
   ✘  Last GitHub CI failed
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/8784891706

RafaelGSS added a commit that referenced this pull request Apr 22, 2024
doc: deprecate --experimental-policy
01c281f 
PR-URL: #52602
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>

RafaelGSS commented Apr 22, 2024

Copy link
Copy Markdown
Member Author

Landed in 01c281f

RafaelGSS closed this Apr 22, 2024
RafaelGSS mentioned this pull request Apr 22, 2024
Merged
RafaelGSS added a commit that referenced this pull request Apr 22, 2024
doc: deprecate --experimental-policy
204018b 
PR-URL: #52602
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
doc: deprecate --experimental-policy
2f6a2e1 
PR-URL: #52602
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
marco-ippolito mentioned this pull request May 2, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
doc: deprecate --experimental-policy
d7434fe 
PR-URL: #52602
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
benjamingr benjamingr approved these changes
joyeecheung joyeecheung approved these changes
MoLow MoLow approved these changes
marco-ippolito marco-ippolito approved these changes
richardlau Awaiting requested review from richardlau
+1 more reviewer
mhdawson mhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. deprecations Issues and PRs related to deprecations. doc Issues and PRs related to the documentations. experimental Issues and PRs related to experimental features. policy Issues and PRs related to the policy subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL