nodejs
repl: runtime deprecate instantiating without new
Followup #54842
This PR runtime-deprecates instantiating the REPL classes without the new keyword.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 88.32%. Comparing base (
f0e5b6a) to head (9bf85bc). Report is 89 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #54869 +/- ##
==========================================
- Coverage 88.43% 88.32% -0.11%
==========================================
Files 654 654
Lines 187662 187673 +11
Branches 36120 36044 -76
==========================================
- Hits 165955 165763 -192
- Misses 14948 15133 +185
- Partials 6759 6777 +18
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/internal/util.js | 97.11% <100.00%> (+0.13%) |
:arrow_up: |
| lib/repl.js | 94.95% <100.00%> (-0.02%) |
:arrow_down: |
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
I think we need a documentation-only deprecation on this first and we need to get a sense of how impactful a runtime deprecation would be on the ecosystem here.
FWIW It has already been doc-deprecated (but very recently).
@jasnell would you be opposed to landing this in v23 entirely-I'm happy to wait until v24?
Got it. While I'd prefer for this to release in v23, I respect your opinion. I'll look into other optimizations in the meantime.
@nodejs/tsc per https://github.com/nodejs/Release/issues/1034
My main question is: is this something that could happen in v23? Or should it wait (like @jasnell suggested)?
Let's give people some time to make arrangements, before polluting their console.
I've added the blocked label, as this should go out with v24.
@jasnell, Putting aside the 'dont release yet', could you re-review?
CITGM (main): https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3492/
CITGM (this PR): https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3494/
Results:
FAILURE: 25 failures in 3494 not present in 3492
┌────────────────────────┬───────────────────┬────────────────────┬──────────────────────┬───────────────────────┬─────────────────────────┬──────────────────────┬──────────────────┬────────────────┐
│ (index) │ 0 │ 1 │ 2 │ 3 │ 4 │ 5 │ 6 │ 7 │
├────────────────────────┼───────────────────┼────────────────────┼──────────────────────┼───────────────────────┼─────────────────────────┼──────────────────────┼──────────────────┼────────────────┤
│ rhel8-ppc64le │ 'mime-v4.0.4' │ │ │ │ │ │ │ │
│ rhel8-x64 │ 'jest-v29.7.0' │ 'mime-v4.0.4' │ 'underscore-v1.13.7' │ │ │ │ │ │
│ osx11-x64 │ 'mime-v4.0.4' │ │ │ │ │ │ │ │
│ osx11 │ 'fastify-v5.0.0' │ 'mime-v4.0.4' │ │ │ │ │ │ │
│ win-vs2022 │ 'resolve-v1.22.8' │ │ │ │ │ │ │ │
│ aix72-ppc64 │ │ │ │ │ │ │ │ │
│ alpine-latest-x64 │ 'jest-v29.7.0' │ 'mime-v4.0.4' │ │ │ │ │ │ │
│ fedora-latest-x64 │ 'jest-v29.7.0' │ 'microtime-v3.1.1' │ 'mime-v4.0.4' │ 'multer-v1.4.5-lts.1' │ 'path-to-regexp-v8.2.0' │ 'serialport-v12.0.0' │ 'sqlite3-v5.1.7' │ 'uuid-v10.0.0' │
│ fedora-last-latest-x64 │ 'jest-v29.7.0' │ 'mime-v4.0.4' │ │ │ │ │ │ │
│ ubuntu2204-64 │ 'jest-v29.7.0' │ 'mime-v4.0.4' │ │ │ │ │ │ │
│ debian12-x64 │ 'jest-v29.7.0' │ 'mime-v4.0.4' │ │ │ │ │ │ │
│ rhel8-s390x │ 'mime-v4.0.4' │ │ │ │ │ │ │ │
└────────────────────────┴───────────────────┴────────────────────┴──────────────────────┴───────────────────────┴─────────────────────────┴──────────────────────┴──────────────────┴────────────────┘
For the failures, all are unrelated (meaning they are caused by things such as Error: getaddrinfo ENOTFOUND github.com or Error: The canary is dead:, etc).
Hey, I assume this needs a new CI since there has been commits since the latest change, can someone start it?
The CITGM shouldn't need a rerun tho (IIUC)
Commit Queue failed
- Loading data for nodejs/node/pull/54869 ✔ Done loading data for nodejs/node/pull/54869 ----------------------------------- PR info ------------------------------------ Title repl: runtime deprecate instantiating without new (#54869) Author Aviv Keller <[email protected]> (@RedYetiDev) Branch RedYetiDev:repl-deprecate-2 -> nodejs:main Labels util, repl, semver-major, author ready, deprecations, needs-ci, needs-citgm, commit-queue-squash Commits 6 - repl: runtime deprecate instantiating without new - fixup! - fixup! fixup! - fixup! fixup! fixup! - fix flaky test - fixup! fix flaky test Committers 2 - RedYetiDev <[email protected]> - GitHub <[email protected]> PR-URL: https://github.com/nodejs/node/pull/54869 Reviewed-By: James M Snell <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/54869 Reviewed-By: James M Snell <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last approving review: ⚠ - repl: runtime deprecate instantiating without new ⚠ - fixup! ⚠ - fixup! fixup! ⚠ - fixup! fixup! fixup! ⚠ - fix flaky test ⚠ - fixup! fix flaky test ℹ This PR was created on Mon, 09 Sep 2024 23:11:31 GMT ✔ Approvals: 4 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/54869#pullrequestreview-2383013809 ✔ - LiviaMedeiros (@LiviaMedeiros): https://github.com/nodejs/node/pull/54869#pullrequestreview-2302928904 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/54869#pullrequestreview-2374242695 ✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/54869#pullrequestreview-2397344156 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-11-03T06:43:31Z: https://ci.nodejs.org/job/node-test-pull-request/63402/ ℹ Last CITGM CI on 2024-10-24T21:16:40Z: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3494/ - Querying data for job/node-test-pull-request/63402/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/11712304862