Skip to content

Conversation

miguelmarcondesf
Copy link
Contributor

Refs #58013

Update util.isArray to eol

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Apr 25, 2025
@geeksilva97 geeksilva97 requested review from jasnell and cjihrig April 25, 2025 19:25
@cjihrig
Copy link
Contributor

cjihrig commented Apr 25, 2025

It seems like there is still some hesitation about doing this - #58013 (comment).

@ljharb ljharb added the needs-citgm PRs that need a CITGM CI run. label Apr 25, 2025
Copy link

codecov bot commented Apr 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.45%. Comparing base (24ded11) to head (98d5b07).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58027      +/-   ##
==========================================
- Coverage   88.45%   88.45%   -0.01%     
==========================================
  Files         703      703              
  Lines      207544   207541       -3     
  Branches    40013    40010       -3     
==========================================
- Hits       183587   183580       -7     
+ Misses      15965    15950      -15     
- Partials     7992     8011      +19     
Files with missing lines Coverage Δ
lib/util.js 97.95% <ø> (-0.02%) ⬇️

... and 37 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@anonrig anonrig added the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 25, 2025
@geeksilva97
Copy link
Contributor

I think build is failing because a link to the util.isArray still exists at https://github.com/nodejs/node/blob/main/doc/api/deprecations.md?plain=1#L998 and https://github.com/nodejs/node/blob/main/doc/api/deprecations.md?plain=1#L4031.

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 27, 2025
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Apr 27, 2025
Copy link
Contributor

Failed to start CI
   ⚠  Commits were pushed since the last approving review:
   ⚠  - util: update util.isArray() to eol
   ⚠  - Update version change with pr number
   ⚠  - fix endoflife typo
   ⚠  - Fix typo from version
   ⚠  - doc: update deprecation notice for util.isArray() removal
   ⚠  - doc: remove unused reference to Array.isArray() in util documentation
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/14686825918

@geeksilva97 geeksilva97 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Apr 28, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 28, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@marco-ippolito
Copy link
Member

CITGM https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3587/

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

I still encounter this API in the wild a fair amount. I'm not all that convinced we should move this one of EOL just yet.

Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

+1 Moving this EOL to Node.js 26.

@miguelmarcondesf
Copy link
Contributor Author

+1 Moving this EOL to Node.js 26.

@RafaelGSS Cool, I'll fix the conflicts today

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

For 26, ok. I still think this one is going to end up a bit more disruptive but that's what semver-major is for.

@geeksilva97 geeksilva97 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 23, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 23, 2025
@nodejs-github-bot
Copy link
Collaborator

@RafaelGSS RafaelGSS added the blocked PRs that are blocked by other issues or PRs. label Sep 23, 2025
@RafaelGSS
Copy link
Member

Let's land it only when v25 goes out. Due to the upcoming V8 update, I might need to sync main with v25.x.

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. blocked PRs that are blocked by other issues or PRs. needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run. semver-major PRs that contain breaking changes and should be released in the next major version. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants