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

doc: deprecate --experimental-policy #52602

Closed
wants to merge 1 commit into from

Conversation

RafaelGSS
Copy link
Member

@RafaelGSS RafaelGSS added the deprecations Issues and PRs related to deprecations. label Apr 19, 2024
@RafaelGSS RafaelGSS marked this pull request as ready for review April 19, 2024 19:30
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 19, 2024
@aduh95
Copy link
Contributor

aduh95 commented Apr 19, 2024

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
Copy link
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
Copy link
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
Copy link
Member

joyeecheung commented Apr 20, 2024

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.

Copy link
Member

@MoLow MoLow left a comment

Choose a reason for hiding this comment

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

+1 on updating the warning

@RafaelGSS RafaelGSS added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 21, 2024
@RedYetiDev RedYetiDev added experimental Issues and PRs related to experimental features. policy Issues and PRs related to the policy subsystem. labels Apr 21, 2024
@RafaelGSS RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 22, 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 Apr 22, 2024
@nodejs-github-bot
Copy link
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

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@RafaelGSS 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 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
Copy link
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
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
Copy link
Member Author

Landed in 01c281f

@RafaelGSS RafaelGSS closed this Apr 22, 2024
RafaelGSS added a commit that referenced this pull request Apr 22, 2024
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
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 3, 2024
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>
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. 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
Development

Successfully merging this pull request may close these issues.