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: Refactor ForEach and destructure the code #49768

Closed
wants to merge 1 commit into from

Conversation

codie-bee
Copy link

@codie-bee codie-bee commented Sep 22, 2023

test: Refactor ForEach and destructure the code

@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 Sep 22, 2023
@Qard Qard added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 22, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 22, 2023
@nodejs-github-bot
Copy link
Collaborator

@Qard
Copy link
Member

Qard commented Sep 22, 2023

The change looks good to me. The commit message lines will need to be 72 characters or less though. You can see the commit message guidelines here: https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#commit-message-guidelines

@codie-bee
Copy link
Author

The change looks good to me. The commit message lines will need to be 72 characters or less though. You can see the commit message guidelines here: https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#commit-message-guidelines

Thanks, updated the commit description

@Qard
Copy link
Member

Qard commented Sep 22, 2023

You will have to push your changes too. Generally to update the commit message you would run git commit --amend to change the message locally, and then git push -f to replace the pushed commit with your modified local changes.

@tniessen tniessen added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Sep 22, 2023
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.

@codie-bee Thanks for the contribution! FYI, the commit email address is not associated with your GitHub account. You can either add the commit email address to your GitHub account, or change your commit email address to your GitHub-associated email address.

@codie-bee
Copy link
Author

@codie-bee Thanks for the contribution! FYI, the commit email address is not associated with your GitHub account. You can either add the commit email address to your GitHub account, or change your commit email address to your GitHub-associated email address.

Thanks @RafaelGSS! I have changed my commit email address.

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.

Almost done! @codie-bee see https://github.com/nodejs/node/actions/runs/6302781305/job/17118196491?pr=49768

You need to adjust the commit message to

- test: Refactor ForEach and destructure the code
+ test: refactor ForEach and destructure the code

@RafaelGSS
Copy link
Member

We are closing it because we have not received any updates after requesting the change.

@RafaelGSS RafaelGSS closed this Nov 24, 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