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: use mustSucceed instead of mustCall #49788

Merged
merged 1 commit into from
Sep 25, 2023
Merged

test: use mustSucceed instead of mustCall #49788

merged 1 commit into from
Sep 25, 2023

Conversation

SiddharthDevulapalli
Copy link
Contributor

@SiddharthDevulapalli SiddharthDevulapalli commented Sep 22, 2023 •
edited
Loading

This pull request is for the Issue provided in the Open Source Day of GHC
Issue guideline - "Replace mustCall((err, data) ...) with mustSucceed((data) ...) in test-trace-events-bootstrap.js"

@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
tniessen
tniessen reviewed Sep 22, 2023
View reviewed changes
Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you @SiddharthDevulapalli. Could you please amend the commit message as described here to comply with our guidelines? The commit message should begin with test:. For example, test: use mustSucceed instead of mustCall would work.

@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
@SiddharthDevulapalli SiddharthDevulapalli changed the title Updated mustCall to mustSucceed test: use mustSucceed instead of mustCall Sep 22, 2023
@SiddharthDevulapalli
Copy link
Contributor Author

Awesome, thank you @SiddharthDevulapalli. Could you please amend the commit message as described here to comply with our guidelines? The commit message should begin with test:. For example, test: use mustSucceed instead of mustCall would work.

@SiddharthDevulapalli
Copy link
Contributor Author

Hi Tobias, I updated the commit title, let me know if this works.

@tniessen
Copy link
Member

@SiddharthDevulapalli Unfortunately, updating the PR title is not sufficient. You need to force-push to the same branch with an amended commit message as described in my previous comment.

@SiddharthDevulapalli
Copy link
Contributor Author

@tniessen I changed the commit message and force pushed, good to go now

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

This comment was marked as outdated.

Sign in to view
@github-actions github-actions bot mentioned this pull request Sep 24, 2023
Open
20 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@tniessen tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 25, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 25, 2023
@nodejs-github-bot nodejs-github-bot merged commit 1716769 into nodejs:main Sep 25, 2023
55 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 1716769

@tniessen
Copy link
Member

Congratulations on becoming a Node.js contributor @SiddharthDevulapalli!

@SiddharthDevulapalli
Copy link
Contributor Author

SiddharthDevulapalli commented Sep 25, 2023 •
edited
Loading

Thank you @tniessen

ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@SiddharthDevulapalli @ruyadorno
test: use mustSucceed instead of mustCall
eded29c 
PR-URL: #49788
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@ruyadorno ruyadorno mentioned this pull request Sep 28, 2023
Closed
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@SiddharthDevulapalli @ruyadorno
test: use mustSucceed instead of mustCall
5ad0042 
PR-URL: #49788
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@ruyadorno ruyadorno mentioned this pull request Sep 28, 2023
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@SiddharthDevulapalli @alexfernandez
test: use mustSucceed instead of mustCall
025eb8d 
PR-URL: nodejs#49788
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
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

@trivikr trivikr trivikr 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
@SiddharthDevulapalli @tniessen @nodejs-github-bot @lpinca @trivikr