Skip to content

Conversation

FabrizioDiPace
Copy link

this PR replace foreach with forof in test-fs-readv.js test-fs-readv-sync.js

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Nov 18, 2023
Copy link
Contributor

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

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

If you are going for performance improvements, I recommend using traditional for loops instead (for (let i = 0; i<wrongInputs.length; i++) { }). Don't forget to run make lint-js

@marco-ippolito marco-ippolito added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 18, 2023
@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 20, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 20, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 15, 2023
@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 Dec 15, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/50787
✔  Done loading data for nodejs/node/pull/50787
----------------------------------- PR info ------------------------------------
Title      test: replace foreach test-fs-readv.js test-fs-readv-sync.js (#50787)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     FabrizioDiPace:main -> nodejs:main
Labels     test, code-and-learn, needs-ci
Commits    2
 - test: replace forEach test-fa-readv-sync
 - Merge branch 'nodejs:main' into main
Committers 2
 - Ospite Privilegiato 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/50787
Reviewed-By: James M Snell 
Reviewed-By: Marco Ippolito 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50787
Reviewed-By: James M Snell 
Reviewed-By: Marco Ippolito 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 18 Nov 2023 16:01:57 GMT
   ✔  Approvals: 3
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/50787#pullrequestreview-1738723027
   ✔  - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/50787#pullrequestreview-1740123948
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/50787#pullrequestreview-1748278556
   ℹ  This PR is being fast-tracked because it is from a Code and Learn event
   ⚠  GitHub cannot link the author of 'test: replace forEach test-fa-readv-sync' to their GitHub account.
   ⚠  Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-11-24T13:50:06Z: https://ci.nodejs.org/job/node-test-pull-request/55889/
- Querying data for job/node-test-pull-request/55889/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 50787
From https://github.com/nodejs/node
 * branch                  refs/pull/50787/merge -> FETCH_HEAD
✔  Fetched commits as 215f4d04b7a9..cfda24f935e3
--------------------------------------------------------------------------------
error: commit cfda24f935e345976044b24b86a5d05045606aa6 is a merge but no -m option was given.
fatal: cherry-pick failed
[main 350ce87c78] test: replace forEach test-fa-readv-sync
 Author: Ospite Privilegiato 
 Date: Sat Nov 18 16:40:37 2023 +0100
 2 files changed, 8 insertions(+), 9 deletions(-)
   ✘  Failed to apply patches
https://github.com/nodejs/node/actions/runs/7223832710

@lpinca
Copy link
Member

lpinca commented Dec 15, 2023

Can you please remove the merge commit? Thank you.

@lpinca lpinca closed this Dec 29, 2023
lpinca pushed a commit that referenced this pull request Dec 29, 2023
Replace `Array.prototype.forEach()` with `for...of` in
`test/parallel/test-fs-readv-sync.js` and
`test/parallel/test-fs-readv.js`.

PR-URL: #50787
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
@lpinca
Copy link
Member

lpinca commented Dec 29, 2023

Landed in ecaf9c3.

RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
Replace `Array.prototype.forEach()` with `for...of` in
`test/parallel/test-fs-readv-sync.js` and
`test/parallel/test-fs-readv.js`.

PR-URL: #50787
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Replace `Array.prototype.forEach()` with `for...of` in
`test/parallel/test-fs-readv-sync.js` and
`test/parallel/test-fs-readv.js`.

PR-URL: #50787
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Replace `Array.prototype.forEach()` with `for...of` in
`test/parallel/test-fs-readv-sync.js` and
`test/parallel/test-fs-readv.js`.

PR-URL: #50787
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. 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.

7 participants