Skip to content

test: replace foreach with for#50599

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
bluescreen:main
Nov 15, 2023
Merged

test: replace foreach with for#50599
nodejs-github-bot merged 1 commit intonodejs:mainfrom
bluescreen:main

Conversation

@bluescreen
Copy link
Contributor

@bluescreen bluescreen commented Nov 7, 2023

nodeconf 2023

@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
@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
@lpinca
Copy link
Member

lpinca commented Nov 8, 2023

Can you please revert unrelated changes?

@bluescreen
Copy link
Contributor Author

bluescreen commented Nov 13, 2023

Can you please revert unrelated changes?

Done

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

nodejs-github-bot commented Nov 13, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/55610/

@bluescreen
Copy link
Contributor Author

bluescreen commented Nov 13, 2023

@lpinca what happened? Looks like the merge is still blocked. Just trying to understand how nodejs CI system actually works.

@lpinca
Copy link
Member

lpinca commented Nov 13, 2023

Unrelated CI failure.

@lpinca
Copy link
Member

lpinca commented Nov 13, 2023 •
edited
Loading

@bluescreen can you please also change "replaced" to "replace" in the commit message? We can do that before landing but it would be easier if you do it as we can simply apply the "commit-queue" label and get the PR merged automatically in that case. Thank you.

@bluescreen
Copy link
Contributor Author

bluescreen commented Nov 13, 2023 •
edited
Loading

@bluescreen can you please also change "replaced" to "replace" in the commit message? We can do that before landing but it would be easier if you do it as we can simply apply the "commit-queue" label and get the PR merged automatically in that case. Thank you.

I squashed the commits and changed the message. Now only the relevant changes made on nodeconf should be in the PR. Hope it's fine for a first learning contribution in nodejs. :)

@github-actions github-actions bot mentioned this pull request Nov 14, 2023
Open
25 tasks
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 14, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 14, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 14, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/55630/

@bluescreen
Copy link
Contributor Author

bluescreen commented Nov 14, 2023

@lpinca what went wrong?

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 14, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/55646/

@github-actions github-actions bot mentioned this pull request Nov 15, 2023
Open
26 tasks
@bluescreen
Copy link
Contributor Author

bluescreen commented Nov 15, 2023

Wohoo it worked! All checks passed. Ready to merge :)

@tniessen tniessen changed the title test: replaced foreach with for-loop test: replace foreach with for Nov 15, 2023
@tniessen tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 15, 2023
@tniessen
Copy link
Member

tniessen commented Nov 15, 2023

Thank you @bluescreen!

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

nodejs-github-bot commented Nov 15, 2023

Landed in ff879cd

targos pushed a commit that referenced this pull request Nov 23, 2023
@bluescreen @targos
test: replace foreach with for
0e8275b 
PR-URL: #50599
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
martenrichter pushed a commit to martenrichter/node that referenced this pull request Nov 26, 2023
@bluescreen @martenrichter
test: replace foreach with for
9c2ddc9 
PR-URL: nodejs#50599
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@RafaelGSS RafaelGSS mentioned this pull request Nov 28, 2023
Merged
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
@bluescreen @UlisesGascon
test: replace foreach with for
37f3ebc 
PR-URL: #50599
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@UlisesGascon UlisesGascon mentioned this pull request Dec 12, 2023
Merged
UlisesGascon pushed a commit that referenced this pull request Dec 19, 2023
@bluescreen @UlisesGascon
test: replace foreach with for
5712c41 
PR-URL: #50599
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes

@tniessen tniessen tniessen approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bluescreen @lpinca @nodejs-github-bot @tniessen @marco-ippolito