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

75 - Replace forEach() and push() with filter() in test-webcrypto-key… #49778

Conversation

SaiHemanth-uab
Copy link

…gen.js [Sai Hemanth Maremalla]

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

Qard commented Sep 22, 2023

Thanks for the contribution! The change looks good, though I think it may fail the lint check for being too long. The commit message also needs to be amended to be less than 72 characters per line. You can edit the commit message with git commit --amend and then git push -f to replace your current pushed changes.

@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

@SaiHemanth-uab SaiHemanth-uab force-pushed the remove_push_forEach_with_filter branch 3 times, most recently from e55800d to 6237ccb Compare September 22, 2023 20:00
@Qard
Copy link
Member

Qard commented Sep 22, 2023

The subsystem in your commit message will need to match one of the subsystems listed in the check here: https://github.com/nodejs/node/actions/runs/6278585916/job/17052917835?pr=49778

I would recommend test.

Also, you can add an additional description to the commit message with an empty line before it. Something like:

test: use filter in test-webcrypto-keygen.js

Replaces forEach and push with filter

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

Thanks for your contribution, could you please adjust the commit message? It must contain the namespace, in your case, test. Therefore, the new message should be:

test: improve test-webcrypto-keygen.js legibility

you can do this by running:

git commit --amend -> adjust your message
git push -f origin remove_push_forEach_with_filter

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.

@SaiHemanth-uab can you update your commit to include your user and email? The easiest way is git config user.name "YOURUSER", git config user.email "yourmail@provider.com" and, git commit --amend --resetAuthor`

@RedYetiDev RedYetiDev added the stalled Issues and PRs that are stalled. label Jun 18, 2024
Copy link
Contributor

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

@RedYetiDev
Copy link
Member

Adding stalled, as the comment was never addressed

@RafaelGSS RafaelGSS closed this Jun 18, 2024
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. stalled Issues and PRs that are stalled. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants