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 foreach test-fs-readv.js test-fs-readv-sync.js #50787

Closed
wants to merge 0 commits into from

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 <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@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 <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@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 <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
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 <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@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