Skip to content

Conversation

@courtnek
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)
Description of change
  • Change vars to let/const
  • Add mustCall
  • equal -> strictEqual
  • remove timeout

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Dec 1, 2016
@imyllerimyller added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Dec 1, 2016
@mscdexmscdex added the repl Issues and PRs related to the REPL subsystem. label Dec 1, 2016
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you line this up with the previous line.

@Trott
Copy link
Member

Ping @courtnek: Thanks for the work on this! Can you make the change requested by @cjihrig and push it to this branch?

If you want, you can copy/paste this into the file:

'use strict';constcommon=require('../common');common.globalCheck=false;constassert=require('assert');constrepl=require('repl');// Create a dummy stream that does nothingconstdummy=newcommon.ArrayStream();functiontestReset(cb){constr=repl.start({input: dummy,output: dummy,useGlobal: false});r.context.foo=42;r.on('reset',common.mustCall(function(context){assert(!!context,'REPL did not emit a context with reset event');assert.strictEqual(context,r.context,'REPL emitted incorrect context');assert.strictEqual(context.foo,undefined,'REPL emitted the previous context, and is not using global as context');context.foo=42;cb();}));r.resetContext();}functiontestResetGlobal(){constr=repl.start({input: dummy,output: dummy,useGlobal: true});r.context.foo=42;r.on('reset',common.mustCall(function(context){assert.strictEqual(context.foo,42,'"foo" property is missing from REPL using global as context');}));r.resetContext();}testReset(common.mustCall(testResetGlobal));

@courtnek
Copy link
ContributorAuthor

Hey @cjihrig@Trott,

Sorry for the delay!

Yeah @Trott I like breaking up the function args over multiple lines over splitting the string. I amended my commit with the new style.

Let me know if there is anything else I can do.

* Change vars to let/const * Add mustCall * equal -> strictEqual * remove timeout
@Trott
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/5519/

@cjihrig Looks like your comment has been addressed. Looks good to you now?

@Trott
Copy link
Member

CI is ✅

@jasnell
Copy link
Member

ping @cjihrig

jasnell pushed a commit that referenced this pull request Dec 24, 2016
* Change vars to let/const * Add mustCall * equal -> strictEqual * remove timeout PR-URL: #9931 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@jasnell
Copy link
Member

Landed in e81e031

@jasnelljasnell closed this Dec 24, 2016
@Trott
Copy link
Member

Thanks for the contribution, @courtnek! 🎉

targos pushed a commit that referenced this pull request Dec 26, 2016
* Change vars to let/const * Add mustCall * equal -> strictEqual * remove timeout PR-URL: #9931 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
targos pushed a commit that referenced this pull request Dec 26, 2016
* Change vars to let/const * Add mustCall * equal -> strictEqual * remove timeout PR-URL: #9931 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This was referenced Dec 27, 2016
@evanlucasevanlucas mentioned this pull request Jan 3, 2017
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
* Change vars to let/const * Add mustCall * equal -> strictEqual * remove timeout PR-URL: #9931 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
* Change vars to let/const * Add mustCall * equal -> strictEqual * remove timeout PR-URL: #9931 Reviewed-By: James M Snell <[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.replIssues and PRs related to the REPL subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@courtnek@Trott@jasnell@cjihrig@mscdex@Fishrock123@imyller@targos@nodejs-github-bot