★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/24430Nouvelle recherche | Portail wanayoo
Skip to content

/node

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: modified order of parameters for assertion #24430

Closed
wants to merge 1 commit into from

Conversation

Reviewers

@thefourtheye thefourtheye

@jasnell jasnell

@targos targos

@cjihrig cjihrig

@gireeshpunathil gireeshpunathil

@BridgeAR BridgeAR

@trivikr trivikr

Assignees
No one assigned
Labels
code-and-learn fast-track test
Projects
None yet
Milestone
No milestone
10 participants
@fiveobjects @targos @gireeshpunathil @Trott @jasnell @thefourtheye @cjihrig @BridgeAR @trivikr @nodejs-github-bot
@fiveobjects
Copy link

fiveobjects commented Nov 17, 2018

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

@nodejs-github-bot nodejs-github-bot added the test label Nov 17, 2018

@BridgeAR

BridgeAR approved these changes Nov 17, 2018
View changes

@BridgeAR BridgeAR added the code-and-learn label Nov 17, 2018

@targos

targos approved these changes Nov 17, 2018
View changes

Copy link
Member

targos left a comment

Changes LGTM.

Commit message should start with test: modify ...
@targos

This comment has been minimized.

Sign in to view
Copy link
Member

targos commented Nov 17, 2018

👍 to fast-track

@targos targos added the fast-track label Nov 17, 2018

@cjihrig

cjihrig approved these changes Nov 17, 2018
View changes

@gireeshpunathil

gireeshpunathil approved these changes Nov 18, 2018
View changes

@gireeshpunathil

This comment has been minimized.

Sign in to view
Copy link
Member

gireeshpunathil commented Nov 18, 2018

@fiveobjects -

  1. please see if you can make the changes that @targos suggested. It is something that we can do while landing; but appreciate if you make an attempt, as doing so strengthens the learning, to the spirit of the code & learn, thanks!

  2. if you'd like you can associate your email with your GitHub account. That way we and GitHub will credit the code contributions you make to you. See https://github.com/settings/emails and the guide at https://help.github.com/articles/setting-your-commit-email-address-on-github/
@trivikr

trivikr approved these changes Nov 18, 2018
View changes

@jasnell

jasnell approved these changes Nov 19, 2018
View changes

@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Nov 19, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/18747/
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Nov 19, 2018

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

thefourtheye approved these changes Nov 20, 2018
View changes

@thefourtheye

thefourtheye reviewed Nov 20, 2018
View changes

test/parallel/test-promises-unhandled-rejections.js
@@ -634,8 +634,8 @@ asyncTest(
const e = new Error('error');
const domainError = new Error('domain error');
onUnhandledSucceed(done, function(reason, promise) {

This comment has been minimized.

Sign in to view
Copy link
@thefourtheye

thefourtheye Nov 20, 2018

Contributor

Nit: A common.mustCall around this would be better.
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Nov 20, 2018

Resume Build: https://ci.nodejs.org/job/node-test-pull-request/18785/
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Nov 20, 2018

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

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Nov 20, 2018

That test-stream-pipeline issue is being pretty stubborn but is also pretty clearly unrelated. Let's do a full CI: https://ci.nodejs.org/job/node-test-pull-request/18815/

Trott added a commit to Trott/io.js that referenced this pull request Nov 20, 2018

Mrityunjoy Saha @Trott
test: modify order of parameters for assertion 
PR-URL: nodejs#24430
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
7ba83e8
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Nov 20, 2018

Landed in 7ba83e8.

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 20, 2018

targos added a commit that referenced this pull request Nov 21, 2018

Mrityunjoy Saha @targos
test: modify order of parameters for assertion 
PR-URL: #24430
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Verified
This commit was signed with a verified signature.
@targos targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
7f76563

rvagg added a commit that referenced this pull request Nov 28, 2018

Mrityunjoy Saha @rvagg
test: modify order of parameters for assertion 
PR-URL: #24430
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
ed714a2

@BridgeAR BridgeAR referenced this pull request Dec 5, 2018

Merged

v11.4.0 proposal #24854

4 of 4 tasks complete

This was referenced Dec 7, 2018

Closed
Closed
Closed
Closed
Closed

codebytere added a commit that referenced this pull request Jan 13, 2019

Mrityunjoy Saha @codebytere
test: modify order of parameters for assertion 
PR-URL: #24430
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Verified
This commit was signed with a verified signature.
@codebytere codebytere Shelley Vohr
GPG key ID: F13993A75599653C Learn about signing commits
e642050

refack added a commit to refack/node that referenced this pull request Jan 14, 2019

Mrityunjoy Saha @refack
test: modify order of parameters for assertion 
PR-URL: nodejs#24430
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
190e250

@codebytere codebytere referenced this pull request Jan 15, 2019

Merged

v10.15.1 proposal #25346

MylesBorins added a commit that referenced this pull request Jan 29, 2019

Mrityunjoy Saha @MylesBorins
test: modify order of parameters for assertion 
PR-URL: #24430
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Verified
This commit was signed with a verified signature.
@MylesBorins MylesBorins Myles Borins
GPG key ID: 933B01F40B5CA946 Learn about signing commits
d5be6ac

codebytere added a commit that referenced this pull request Jan 29, 2019

Mrityunjoy Saha @codebytere
test: modify order of parameters for assertion 
PR-URL: #24430
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Verified
This commit was signed with a verified signature.
@codebytere codebytere Shelley Vohr
GPG key ID: F13993A75599653C Learn about signing commits
e7d41c0

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
You can’t perform that action at this time.