Skip to content

Conversation

john-mcinall
Copy link
Contributor

No description provided.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@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 7, 2023
@ljharb
Copy link
Member

ljharb commented Nov 7, 2023

for-of is more brittle than forEach; why this change?

@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 7, 2023
@RafaelGSS
Copy link
Member

for-of is more brittle than forEach; why this change?

This is for a workshop we're running at NodeConf. See the code-and-learn label.

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.

LGTM

@ljharb
Copy link
Member

ljharb commented Nov 7, 2023

Alrighty; it’d still be good to have a justification provided :-)

@RafaelGSS RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 7, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 7, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@benjamingr
Copy link
Member

for-of is more brittle than forEach; why this change?

Note this is a test file so that shouldn't be a concern

@ljharb
Copy link
Member

ljharb commented Nov 8, 2023

I totally agree; I’m just surprised that a learning exercise doesn’t include instructions to provide a justification.

@nodejs-github-bot
Copy link
Collaborator

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 10, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 10, 2023
@nodejs-github-bot nodejs-github-bot merged commit fd4a305 into nodejs:main Nov 10, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in fd4a305

targos pushed a commit that referenced this pull request Nov 11, 2023
PR-URL: #50602
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Nov 14, 2023
PR-URL: #50602
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #50602
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Dec 12, 2023
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. 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