Skip to content

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

Closed
blacksun1 wants to merge 1 commit into
nodejs:masterfrom
blacksun1:master
Closed

test: remove unused function arguments in async-hooks tests#24406
blacksun1 wants to merge 1 commit into
nodejs:masterfrom
blacksun1:master

Conversation

@blacksun1

@blacksun1 blacksun1 commented Nov 16, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 Node.js core tests and test infrastructure. labels Nov 16, 2018
@blacksun1 blacksun1 changed the title chore: cleanup of async-hooks JS tests Nov 16, 2018
@blacksun1 blacksun1 changed the title async-hooks: cleanup of tests Nov 16, 2018
@Trott

Trott commented Nov 17, 2018

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/18696/
refack
refack approved these changes Nov 17, 2018
View reviewed changes
Comment thread test/async-hooks/test-promise.js Outdated
@refack refack added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Nov 17, 2018
@refack

refack commented Nov 17, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

blacksun1 commented Nov 17, 2018

Copy link
Copy Markdown
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

blacksun1 commented Nov 17, 2018

Copy link
Copy Markdown
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

Trott commented Nov 17, 2018

Copy link
Copy Markdown
Member

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

blacksun1 commented Nov 18, 2018

Copy link
Copy Markdown
Contributor Author

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

Trott commented Nov 19, 2018

Copy link
Copy Markdown
Member

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

Trott commented Nov 19, 2018

Copy link
Copy Markdown
Member

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

Labels

async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs with CI started, the required approvals, and no outstanding review comments. test Issues and PRs related to Node.js core tests and test infrastructure.

6 participants

@blacksun1 @Trott @refack @jasnell @starkwang @nodejs-github-bot