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.

Sign up for GitHub

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

Jump to bottom

test: replace foreach with for #50599

Merged
merged 1 commit into from
Nov 15, 2023
Merged

test: replace foreach with for #50599

merged 1 commit into from
Nov 15, 2023

Conversation

bluescreen
Copy link
Contributor

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

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

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

@bluescreen
Copy link
Contributor Author

@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

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

@bluescreen
Copy link
Contributor Author

@lpinca what went wrong?

@nodejs-github-bot
Copy link
Collaborator

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

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

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
53 checks passed
@nodejs-github-bot
Copy link
Collaborator

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 <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
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 <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@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 <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@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 <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tniessen tniessen tniessen approved these changes

@lpinca lpinca lpinca 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