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: remove unused function arguments in async-hooks tests #24406

Closed
wants to merge 1 commit into from
Closed

test: remove unused function arguments in async-hooks tests #24406

wants to merge 1 commit into from

Conversation

blacksun1
Copy link
Contributor

@blacksun1 blacksun1 commented Nov 16, 2018 •
edited
Loading

Remove unused function arguments in three async-hooks tests and improve test consistency.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to the tests. labels Nov 16, 2018
@blacksun1 blacksun1 changed the title chore: cleanup of async-hooks JS tests async-hooks: cleanup of tests Nov 16, 2018
@blacksun1 blacksun1 changed the title async-hooks: cleanup of tests test: remove unused function arguments in async-hooks tests Nov 16, 2018
@Trott
Copy link
Member

Trott commented Nov 17, 2018

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

refack
refack approved these changes Nov 17, 2018
View reviewed changes
test/async-hooks/test-promise.js Show resolved Hide resolved
@refack refack added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 17, 2018
@refack
Copy link
Contributor

refack commented Nov 17, 2018 •
edited
Loading

Hello @blacksun1, and thank you for the contribution🥇
[Canned greeting to first-time contributors]
If you are not familiar with our review and landing process, it's covered in CONTRIBUTING.md
[end can]

P.S. But seriously, if you have any questions, feel free to contact me directly.

P.P.S. I see you have two GitHub users, take a look at the headers of https://patch-diff.githubusercontent.com/raw/nodejs/node/pull/24406.patch and make sure the author attribution is to you liking.

@blacksun1
Copy link
Contributor Author

P.P.S. I see you have two GitHub users, take a look at the headers of https://patch-diff.githubusercontent.com/raw/nodejs/node/pull/24406.patch and make sure the author attribution is to you liking.

Shoot, I saw that the email address was wrong. I've updated the commit and repushed. I hope that this was the correct way to make sure it was set correctly.

@blacksun1
Copy link
Contributor Author

P.P.S. I see you have two GitHub users, take a look at the headers of https://patch-diff.githubusercontent.com/raw/nodejs/node/pull/24406.patch and make sure the author attribution is to you liking.

Looks correct now

@Trott
Copy link
Member

Trott commented Nov 17, 2018

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

@blacksun1
test: remove unused function arguments in async-hooks tests
336ec08 
Remove unused function arguments in three async-hooks tests and
improve test consistancy.
@blacksun1
Copy link
Contributor Author

@Trott, do you want to run your CI job again? Not sure why it crashed last time

@Trott
Copy link
Member

Trott commented Nov 19, 2018

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/18733/

Trott pushed a commit to Trott/io.js that referenced this pull request Nov 19, 2018
@blacksun1 @Trott
test: remove unused function arguments in async-hooks tests
738e076 
Remove unused function arguments in three async-hooks tests and
improve test consistancy.

PR-URL: nodejs#24406
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member

Trott commented Nov 19, 2018

Landed in 738e076.

Thanks for the contribution! 🎉

(If you're interested in other possible contributions to Node.js but don't have a good idea of where to start looking, some ideas are posted at https://www.nodetodo.org/next-steps/.)

@Trott Trott closed this Nov 19, 2018
targos pushed a commit that referenced this pull request Nov 19, 2018
@blacksun1 @targos
test: remove unused function arguments in async-hooks tests
aea235a 
Remove unused function arguments in three async-hooks tests and
improve test consistancy.

PR-URL: #24406
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
@blacksun1 @rvagg
test: remove unused function arguments in async-hooks tests
2777bc4 
Remove unused function arguments in three async-hooks tests and
improve test consistancy.

PR-URL: #24406
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Dec 5, 2018
Merged
4 tasks
This was referenced Dec 7, 2018
Closed
Closed
Closed
Closed
Closed
codebytere pushed a commit that referenced this pull request Jan 13, 2019
@blacksun1 @codebytere
test: remove unused function arguments in async-hooks tests
63735da 
Remove unused function arguments in three async-hooks tests and
improve test consistancy.

PR-URL: #24406
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@blacksun1 @refack
test: remove unused function arguments in async-hooks tests
1859285 
Remove unused function arguments in three async-hooks tests and
improve test consistancy.

PR-URL: nodejs#24406
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@codebytere codebytere mentioned this pull request Jan 15, 2019
Merged
codebytere pushed a commit that referenced this pull request Jan 29, 2019
@blacksun1 @codebytere
test: remove unused function arguments in async-hooks tests
2d25cdd 
Remove unused function arguments in three async-hooks tests and
improve test consistancy.

PR-URL: #24406
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced Jan 29, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@starkwang starkwang starkwang approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@blacksun1 @Trott @refack @jasnell @starkwang @nodejs-github-bot