Skip to content

Conversation

@NoSkillGirl
Copy link
Contributor

In test-net-write-callback.js, when the process exits, we check of callback
count with the expected value. written assert.strictEqual(N, cbcount),
which expects N to equal to cbcount, instead we should expect cbcount to be equal to cbcount.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

In test-net-write-callback.js, when process exits, we check of callback count with the expected value. written assert.strictEqual(N, cbcount), which expects N to equal to cbcount, instead we should expect cbcount to be equal to cbcount.
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Nov 17, 2018
@gireeshpunathilgireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 17, 2018
Copy link
Member

@targostargos left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

Commit message should look like test: fix ...

@targos
Copy link
Member

👍 to fast-track

@targostargos added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 17, 2018
@Trott
Copy link
Member

Trott pushed a commit to Trott/io.js that referenced this pull request Nov 19, 2018
In test-net-write-callback.js, when process exits, we check callback count against the expected value. The assertion is written with the expected value first and actual value second, but that is the opposite of the documented argument order. Reverse them to be consistent with documentation. PR-URL: nodejs#24422 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@Trott
Copy link
Member

Landed in dafd76c. (I edited the commit message.)

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/.)

@TrottTrott closed this Nov 19, 2018
targos pushed a commit that referenced this pull request Nov 19, 2018
In test-net-write-callback.js, when process exits, we check callback count against the expected value. The assertion is written with the expected value first and actual value second, but that is the opposite of the documented argument order. Reverse them to be consistent with documentation. PR-URL: #24422 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@NoSkillGirl
Copy link
ContributorAuthor

Hi @gireeshpunathil, Is this pull request merged or closed ? @Trott edited the commit message and the PR got reviewed by multiple people. @Trott kept a "Thanks for the contribution!" message But i see the PR got closed with unmerged commits.

@Trott
Copy link
Member

@NoSkillGirl The "closed" is an artifact of the way I merged the commit, but it's really there in master. See dafd76c.

rvagg pushed a commit that referenced this pull request Nov 28, 2018
In test-net-write-callback.js, when process exits, we check callback count against the expected value. The assertion is written with the expected value first and actual value second, but that is the opposite of the documented argument order. Reverse them to be consistent with documentation. PR-URL: #24422 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@BridgeARBridgeAR mentioned this pull request Dec 5, 2018
4 tasks
codebytere pushed a commit that referenced this pull request Jan 13, 2019
In test-net-write-callback.js, when process exits, we check callback count against the expected value. The assertion is written with the expected value first and actual value second, but that is the opposite of the documented argument order. Reverse them to be consistent with documentation. PR-URL: #24422 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
In test-net-write-callback.js, when process exits, we check callback count against the expected value. The assertion is written with the expected value first and actual value second, but that is the opposite of the documented argument order. Reverse them to be consistent with documentation. PR-URL: nodejs#24422 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@codebyterecodebytere mentioned this pull request Jan 15, 2019
codebytere pushed a commit that referenced this pull request Jan 29, 2019
In test-net-write-callback.js, when process exits, we check callback count against the expected value. The assertion is written with the expected value first and actual value second, but that is the opposite of the documented argument order. Reverse them to be consistent with documentation. PR-URL: #24422 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learnIssues related to the Code-and-Learn events and PRs submitted during the events.fast-trackPRs that do not need to wait for 48 hours to land.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@NoSkillGirl@targos@Trott@cjihrig@oyyd@gireeshpunathil@trivikr@nodejs-github-bot