Skip to content

doc: deprecate --experimental-policy#52602

Closed
RafaelGSS wants to merge 1 commit intonodejs:mainfrom
RafaelGSS:doc-deprecate-policy
Closed

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

Conversation

@RafaelGSS
Copy link
Member

@RafaelGSS RafaelGSS commented Apr 19, 2024

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

nodejs-github-bot commented Apr 19, 2024

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

richardlau commented Apr 19, 2024

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

marco-ippolito commented Apr 20, 2024

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 joyeecheung mentioned this pull request Apr 20, 2024
Merged
@joyeecheung
Copy link
Member

joyeecheung commented Apr 20, 2024 •
edited
Loading

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
MoLow approved these changes Apr 21, 2024
View reviewed changes
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
@avivkeller avivkeller 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

nodejs-github-bot commented Apr 22, 2024

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
mhdawson approved these changes Apr 22, 2024
View reviewed changes
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

nodejs-github-bot commented Apr 22, 2024

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

RafaelGSS commented Apr 22, 2024

Landed in 01c281f

@RafaelGSS RafaelGSS closed this Apr 22, 2024
@RafaelGSS RafaelGSS mentioned this pull request Apr 22, 2024
Merged
RafaelGSS added a commit that referenced this pull request Apr 22, 2024
@RafaelGSS
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
@RafaelGSS @marco-ippolito
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 marco-ippolito mentioned this pull request May 2, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@RafaelGSS @marco-ippolito
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@benjamingr benjamingr benjamingr approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@MoLow MoLow MoLow approved these changes

@mhdawson mhdawson mhdawson approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

@richardlau richardlau Awaiting requested review from richardlau

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