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

test: replace .substr with .slice #53070

Merged
merged 1 commit into from
May 22, 2024
Merged

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented May 20, 2024

String.prototype.substr is deprecated, and using it will raise an error when using ESLint 9+ (#52780).

`String.prototype.substr` is deprecated, and using it will raise an
error when using ESLint 9+.

Co-authored-by: =?UTF-8?q?Micha=C3=ABl=20Zasso?= <[email protected]>
@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 20, 2024
@aduh95 aduh95 added request-ci Add this label to start a Jenkins CI on a PR. test Issues and PRs related to the tests. needs-ci PRs that need a full CI run. and removed test Issues and PRs related to the tests. esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels May 20, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 20, 2024
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

It's not exactly ESLint 9 that introduces the error, but the migration to flat config.

Copy link
Member

@juanarbol juanarbol left a comment

Choose a reason for hiding this comment

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

lgtm

Just curiosity, why not substring?

@targos
Copy link
Member

targos commented May 21, 2024

I return the question: why do you think we should use substring?

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels May 21, 2024
@targos targos added the fast-track PRs that do not need to wait for 48 hours to land. label May 21, 2024
Copy link
Contributor

Fast-track has been requested by @targos. Please 👍 to approve.

@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 May 21, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/53070
✔  Done loading data for nodejs/node/pull/53070
----------------------------------- PR info ------------------------------------
Title      test: replace `.substr` with `.slice` (#53070)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     aduh95:sub-substr -> nodejs:main
Labels     test, fast-track, author ready, needs-ci
Commits    1
 - test: replace `.substr` with `.slice`
Committers 1
 - Antoine du Hamel 
PR-URL: https://github.com/nodejs/node/pull/53070
Reviewed-By: Michaël Zasso 
Reviewed-By: Trivikram Kamat 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Luigi Pinca 
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Juan José Arboleda 
Reviewed-By: Moshe Atlow 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53070
Reviewed-By: Michaël Zasso 
Reviewed-By: Trivikram Kamat 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Luigi Pinca 
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Juan José Arboleda 
Reviewed-By: Moshe Atlow 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 20 May 2024 11:21:02 GMT
   ✔  Approvals: 7
   ✔  - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/53070#pullrequestreview-2066181127
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/53070#pullrequestreview-2066321383
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/53070#pullrequestreview-2066494050
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/53070#pullrequestreview-2066592235
   ✔  - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/53070#pullrequestreview-2067271445
   ✔  - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/53070#pullrequestreview-2067536710
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/53070#pullrequestreview-2067888793
   ℹ  This PR is being fast-tracked
   ✘  This PR needs to wait 22 more hours to land (or 0 hours if there is 1 more approval (👍) of the fast-track request from collaborators).
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-05-20T11:26:29Z: https://ci.nodejs.org/job/node-test-pull-request/59311/
- Querying data for job/node-test-pull-request/59311/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/9174979544

@bnb bnb 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 May 21, 2024
@bnb
Copy link
Contributor

bnb commented May 21, 2024

requeued (?) since it was only missing a 👍🏻 on the fast track

@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 May 21, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/53070
✔  Done loading data for nodejs/node/pull/53070
----------------------------------- PR info ------------------------------------
Title      test: replace `.substr` with `.slice` (#53070)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     aduh95:sub-substr -> nodejs:main
Labels     test, fast-track, author ready, needs-ci
Commits    1
 - test: replace `.substr` with `.slice`
Committers 1
 - Antoine du Hamel 
PR-URL: https://github.com/nodejs/node/pull/53070
Reviewed-By: Michaël Zasso 
Reviewed-By: Trivikram Kamat 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Luigi Pinca 
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Juan José Arboleda 
Reviewed-By: Moshe Atlow 
Reviewed-By: Tierney Cyren 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53070
Reviewed-By: Michaël Zasso 
Reviewed-By: Trivikram Kamat 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Luigi Pinca 
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Juan José Arboleda 
Reviewed-By: Moshe Atlow 
Reviewed-By: Tierney Cyren 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 20 May 2024 11:21:02 GMT
   ✔  Approvals: 8
   ✔  - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/53070#pullrequestreview-2066181127
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/53070#pullrequestreview-2066321383
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/53070#pullrequestreview-2066494050
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/53070#pullrequestreview-2066592235
   ✔  - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/53070#pullrequestreview-2067271445
   ✔  - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/53070#pullrequestreview-2067536710
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/53070#pullrequestreview-2067888793
   ✔  - Tierney Cyren (@bnb): https://github.com/nodejs/node/pull/53070#pullrequestreview-2069078323
   ℹ  This PR is being fast-tracked
   ✘  GitHub CI is still running
   ℹ  Last Full PR CI on 2024-05-21T12:51:22Z: https://ci.nodejs.org/job/node-test-pull-request/59311/
- Querying data for job/node-test-pull-request/59311/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/9177998933

@targos targos 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 May 22, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 22, 2024
@nodejs-github-bot nodejs-github-bot merged commit 4a54a80 into nodejs:main May 22, 2024
90 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 4a54a80

@Mortiemi

This comment was marked as off-topic.

@aduh95 aduh95 deleted the sub-substr branch May 22, 2024 13:17
targos added a commit that referenced this pull request Jun 1, 2024
`String.prototype.substr` is deprecated, and using it will raise an
error when using ESLint 9+.

Co-authored-by: =?UTF-8?q?Micha=C3=ABl=20Zasso?= <[email protected]>
PR-URL: #53070
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
lukins-cz pushed a commit to lukins-cz/OS-Aplet-node that referenced this pull request Jun 1, 2024
`String.prototype.substr` is deprecated, and using it will raise an
error when using ESLint 9+.

Co-authored-by: =?UTF-8?q?Micha=C3=ABl=20Zasso?= <[email protected]>
PR-URL: nodejs#53070
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
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. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet