Skip to content

nina 2016 code-n-learn, issue: use assert.Equal() #9977

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed

Conversation

ianwhitedeveloper
Copy link
Contributor

@ianwhitedeveloper ianwhitedeveloper commented Dec 1, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

Test

Description of change

test: change assert.equal to assert.strictEqual

the following lines have been updated:

27:1
37:1
49:1
61:3
66:1
79:1

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 1, 2016
@imyller
Copy link
Member

imyller commented Dec 1, 2016

@ianwhitedeveloper May I kindly ask you to format the commit message as described in CONTRIBUTING guidelines.

@imyller imyller added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Dec 1, 2016
@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Dec 1, 2016
@ianwhitedeveloper
Copy link
Contributor Author

ianwhitedeveloper commented Dec 1, 2016

mad bad @imyller and @mscdex. I have updated my commit message. Thanks for taking a look!
Also, I'm sharing a computer with a co-worker. Will have him push an amended commit ASAP.

@ianwhitedeveloper
Copy link
Contributor Author

Alrighty just pushed an amended commit. Thanks again.

@imyller
Copy link
Member

imyller commented Dec 1, 2016

@ianwhitedeveloper Looks great! Thank you 👍

@gibfahn
Copy link
Member

gibfahn commented Dec 1, 2016

@ianwhitedeveloper FYI, your commit has the name cdnadmin (email: cdnadmin@collaborare.net). If you want to change that to your name and email, you probably want to do something like:

git commit --amend --no-edit --author="Ian White <ian.white.developer@gmail.com>"

And then update your branch (which will update this PR).

@@ -34,7 +34,7 @@ fs.appendFileSync(filename2, data);

var fileData2 = fs.readFileSync(filename2);

assert.equal(Buffer.byteLength(data) + currentFileData.length,
assert.strictEqual(Buffer.byteLength(data) + currentFileData.length,
fileData2.length);
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be lined up now.

}

var fileData4 = fs.readFileSync(filename4);

assert.equal(Buffer.byteLength('' + num) + currentFileData.length,
assert.strictEqual(Buffer.byteLength('' + num) + currentFileData.length,
fileData4.length);
Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

@@ -76,7 +76,7 @@ fs.closeSync(filename5fd);

var fileData5 = fs.readFileSync(filename5);

assert.equal(Buffer.byteLength(data) + currentFileData.length,
assert.strictEqual(Buffer.byteLength(data) + currentFileData.length,
fileData5.length);
Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

@ianwhitedeveloper
Copy link
Contributor Author

Alrighty @cjihrig I'll make those updates ASAP. Having problems with my computer so I'm going to need to clear a bunch of space to get the project up and running, but it's on my radar! thanks for taking a look.

@ianwhitedeveloper
Copy link
Contributor Author

Ok @cjihrig I made the changes. Hopefully I didn't misunderstand the problem! Thanks again!

Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

Thanks! Code changes LGTM.

@cjihrig
Copy link
Contributor

cjihrig commented Dec 2, 2016

Trott pushed a commit to Trott/io.js that referenced this pull request Dec 4, 2016
Change instances of `asset.equal` to `assert.strictEqual`.

PR-URL: nodejs#9977
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott
Copy link
Member

Trott commented Dec 4, 2016

Landed in 62f4b77. Thanks for the contribution! 🎉

@Trott Trott closed this Dec 4, 2016
addaleax pushed a commit that referenced this pull request Dec 5, 2016
Change instances of `asset.equal` to `assert.strictEqual`.

PR-URL: #9977
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
2 tasks
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
Change instances of `asset.equal` to `assert.strictEqual`.

PR-URL: nodejs#9977
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
Change instances of `asset.equal` to `assert.strictEqual`.

PR-URL: nodejs#9977
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
Change instances of `asset.equal` to `assert.strictEqual`.

PR-URL: #9977
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 21, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants