test_runner: replace forEach() with for ... of#50595
test_runner: replace forEach() with for ... of#50595nodejs-github-bot merged 1 commit intonodejs:mainfrom
Conversation
|
Review requested:
|
|
why? assert.forEach isn’t the same as a for..of loop. |
853f68e to
18bd8aa
Compare
18bd8aa to
d907f14
Compare
atlowChemi
left a comment
There was a problem hiding this comment.
Hi @tomhaddad, what is the reason for this change?
This is not a part of core in which we avoid prototype calls like forEach()
|
@atlowChemi it's a code-and-learn exercise. |
|
That doesn’t mean the change should just land tho, right? Or does it mean the PR isn’t going to actually land, it’s just to demonstrate how to do one? |
aduh95
left a comment
There was a problem hiding this comment.
Note to reviewers: this is a test file, we do not have any requirements for how to iterate over arrays in test files, both methods are fine by me.
As any PR, this can land after if it has approval and green CI, after the waiting time. Code and Learn consists on making low hanging fruit PRs to get a first contribution to the repo, with the hope that it will result in more contributions in the future. Thanks for the PR @tomhaddad :) |
You are most welcome, happy to have made my first contribution :) |
|
Fast-track has been requested by @aduh95. Please 👍 to approve. |
|
Landed in 8588ee1 |
PR-URL: #50595 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
PR-URL: #50595 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
PR-URL: #50595 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Replaces usage of forEach() with for ... of in test-runner-mock-timers.js
#NodeConf