Skip to content

Conversation

@kylecarter
Copy link
Contributor

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
  • Updated references of var to const
  • Updated assert.equal to assert.strictEqual
  • Fixed eslint error for multiline string on line 12

- Updated references of var to const
- Updated assert.equal to assert.strictEqual
- Fixed eslint error for multiline string on line 12.
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 1, 2016
@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Dec 1, 2016
@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
Copy link
Contributor

mscdex commented Dec 1, 2016

Subsystem name should be all lowercase. Also, a minor nit: the trailing period at the end of the first line of the commit message is unnecessary.

@Trott
Copy link
Member

Trott commented Dec 4, 2016

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM if CI is ✅

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Actually, I take it back. The change to the multi-line string on line 12 would seem to change the string by introducing a lot more whitespace, unless I'm missing something... It is also contrary to the conventions throughout the rest of the code base.

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

CI is green. I'll undo the whitespace/string thing on landing. LGTM. Landing!

Trott pushed a commit to Trott/io.js that referenced this pull request Dec 4, 2016
- Updated references of var to const
- Updated assert.equal to assert.strictEqual

PR-URL: nodejs#9992
Reviewed-By: Rich Trott <[email protected]>
@Trott
Copy link
Member

Trott commented Dec 4, 2016

Landed in 451f88e. Thanks for the contribution! 🎉

@Trott Trott closed this Dec 4, 2016
addaleax pushed a commit that referenced this pull request Dec 5, 2016
- Updated references of var to const
- Updated assert.equal to assert.strictEqual

PR-URL: #9992
Reviewed-By: Rich Trott <[email protected]>
@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
- Updated references of var to const
- Updated assert.equal to assert.strictEqual

PR-URL: nodejs#9992
Reviewed-By: Rich Trott <[email protected]>
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
- Updated references of var to const
- Updated assert.equal to assert.strictEqual

PR-URL: nodejs#9992
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
- Updated references of var to const
- Updated assert.equal to assert.strictEqual

PR-URL: #9992
Reviewed-By: Rich Trott <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

7 participants