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.

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: update equal to strictEqual & function to arrow function #8596

Closed
wants to merge 2 commits into from

Conversation

digitalsadhu
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change
  • update 4 instances of equal to strictEqual
  • 1 instance of function to arrow function syntax

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 17, 2016
@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Sep 17, 2016
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.

LGTM

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM with one suggestion

@@ -11,12 +11,12 @@ fs.read(fd,
expected.length,
0,
'utf-8',
common.mustCall(function(err, str, bytesRead) {
common.mustCall((err, str, bytesRead) => {
assert.ok(!err);
Copy link
Member

Choose a reason for hiding this comment

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

This could be assert.ifError(err);, which gives slightly more information when anything fails.

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM. +1 to @addaleax's comment regarding using assert.ifError()

@digitalsadhu
Copy link
Contributor Author

Changed .ok to .ifError as suggested. Ready for re-review.

@jasnell
Copy link
Member

jasnell commented Sep 20, 2016

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM

jasnell pushed a commit that referenced this pull request Sep 22, 2016
* Replace equal with strictEqual
* Update functions to arrow functions

PR-URL: #8596
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@jasnell
Copy link
Member

jasnell commented Sep 22, 2016

Landed in 0718dd9. thank you!

@jasnell jasnell closed this Sep 22, 2016
jasnell pushed a commit that referenced this pull request Sep 29, 2016
* Replace equal with strictEqual
* Update functions to arrow functions

PR-URL: #8596
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
* Replace equal with strictEqual
* Update functions to arrow functions

PR-URL: #8596
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

8 participants