test: refactor the code in test-util-debug.js #10531

sivaprs · 2016-12-30T05:35:51Z

use const and let instead of var
use assert.strictEqual instead of assert.equal

Checklist

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

Affected core subsystem(s)

test

Description

use const and let instead of var
use assert.strictEqual instead of assert.equal

sivaprs · 2016-12-30T05:36:57Z

cc @thefourtheye

lpinca · 2016-12-30T10:37:31Z

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

gibfahn

LGTM pending CI

gibfahn · 2016-12-30T11:05:43Z

FYI your Github name is set to sivaprasanna, people usually tend to use their full names (e.g. @thefourtheye here). If you want to change it the commands are:

# To change it everywhere:
git config --global user.name "Siva Prasanna"

# To change it for this one commit:
git commit --amend --no-edit  --author="Siva Prasanna <[email protected]>"

Whoever lands this Pull Request can do it for you as part of the landing process instead if you'd like. If you don't want to change it that's totally fine too!

thefourtheye · 2017-01-02T07:35:50Z

test/sequential/test-util-debug.js

Can you remove all the console.logs while you are here?

sivaprs · 2017-01-02T12:59:52Z

cc @thefourtheye

thefourtheye · 2017-01-02T13:17:41Z

test/sequential/test-util-debug.js

You missed one here :)

console.log('ok') is expected to be there because it has check at line no. 49

Awww, right...

cjihrig · 2017-01-03T14:19:46Z

@sivaprs, it looks like something went wrong during the rebase process. This is now 17 commits.

edsadr · 2017-01-03T16:24:17Z

test/sequential/test-util-debug.js

Please change anonymous functions to arrow functions

edsadr

Please change anonymous functions to arrow functions

edsadr · 2017-01-03T16:24:28Z

test/sequential/test-util-debug.js

Please change anonymous functions to arrow functions

edsadr · 2017-01-03T16:24:47Z

test/sequential/test-util-debug.js

Please change anonymous functions to arrow functions

gibfahn · 2017-01-03T16:32:06Z

@edsadr I don't think changing anonymous functions to arrow functions is required, you can do it if you want, but it's not actually necessary.

edsadr · 2017-01-03T16:42:10Z

@gibfahn is just a small Nit now that @sivaprs is touching the file would be a nice thing to add ... I had the same file in another PR including that... closed it in favor of this one, not a big deal anyway 🙂

* use const and let instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * removed unwanted console log

sivaprs · 2017-01-04T06:30:59Z

@cjihrig I rebased now. PTAL.

thefourtheye · 2017-01-04T06:34:00Z

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

* use const and let instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * removed unwanted console log PR-URL: #10531 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>

jasnell · 2017-01-06T17:52:01Z

Landed in 8839d50

* use const and let instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * removed unwanted console log PR-URL: nodejs#10531 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>

* use const and let instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * removed unwanted console log PR-URL: #10531 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>

* use const and let instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * removed unwanted console log PR-URL: nodejs#10531 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>

jasnell · 2017-03-08T20:29:32Z

This will need backport URLs for v4 and v6 to land there.

nodejs-github-bot added test Issues and PRs related to the tests. lts-watch-v6.x labels Dec 30, 2016

mscdex added the util Issues and PRs related to the built-in util module. label Dec 30, 2016

lpinca approved these changes Dec 30, 2016

View reviewed changes

gibfahn approved these changes Dec 30, 2016

View reviewed changes

cjihrig approved these changes Dec 30, 2016

View reviewed changes

jasnell approved these changes Dec 30, 2016

View reviewed changes

thefourtheye approved these changes Jan 2, 2017

View reviewed changes

test/sequential/test-util-debug.js Outdated

Copy link

Contributor

thefourtheye Jan 2, 2017

Choose a reason for hiding this comment

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

Can you remove all the console.logs while you are here?

nodejs-github-bot added the dont-land-on-v7.x label Jan 2, 2017

thefourtheye reviewed Jan 2, 2017

View reviewed changes

nodejs-github-bot added the dont-land-on-v7.x label Jan 3, 2017

thefourtheye mentioned this pull request Jan 3, 2017

test: improve the code in test-util-debug.js #10583

Closed

3 tasks

edsadr reviewed Jan 3, 2017

View reviewed changes

test/sequential/test-util-debug.js Outdated

Copy link

Member

edsadr Jan 3, 2017

Choose a reason for hiding this comment

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

Please change anonymous functions to arrow functions

edsadr reviewed Jan 3, 2017

View reviewed changes

test: refactor the code in test-util-debug.js

40921a5

* use const and let instead of var * use assert.strictEqual instead of assert.equal * use arrow functions * removed unwanted console log

sivaprs force-pushed the test-2016 branch from 40eb34c to 40921a5 Compare January 4, 2017 06:29

nodejs-github-bot added the dont-land-on-v7.x label Jan 4, 2017

jasnell closed this Jan 6, 2017

mscdex removed the dont-land-on-v7.x label Jan 13, 2017

italoacasas mentioned this pull request Jan 25, 2017

Proposal to update v7.x-staging #10989

Closed

italoacasas mentioned this pull request Jan 29, 2017

v7.5.0 proposal #11062

Merged

jasnell added dont-land-on-v4.x labels Mar 8, 2017

Uh oh!

test: refactor the code in test-util-debug.js #10531

test: refactor the code in test-util-debug.js #10531

Uh oh!

Conversation

sivaprs commented Dec 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Uh oh!

sivaprs commented Dec 30, 2016

Uh oh!

lpinca commented Dec 30, 2016

Uh oh!

gibfahn left a comment

Choose a reason for hiding this comment

Uh oh!

gibfahn commented Dec 30, 2016

Uh oh!

thefourtheye Jan 2, 2017

Choose a reason for hiding this comment

Uh oh!

sivaprs commented Jan 2, 2017

Uh oh!

thefourtheye Jan 2, 2017

Choose a reason for hiding this comment

Uh oh!

sivaprs Jan 3, 2017

Choose a reason for hiding this comment

Uh oh!

thefourtheye Jan 3, 2017

Choose a reason for hiding this comment

Uh oh!

cjihrig commented Jan 3, 2017

Uh oh!

edsadr Jan 3, 2017

Choose a reason for hiding this comment

Uh oh!

edsadr left a comment

Choose a reason for hiding this comment

Uh oh!

edsadr Jan 3, 2017

Choose a reason for hiding this comment

Uh oh!

edsadr Jan 3, 2017

Choose a reason for hiding this comment

Uh oh!

gibfahn commented Jan 3, 2017

Uh oh!

edsadr commented Jan 3, 2017

Uh oh!

sivaprs commented Jan 4, 2017

Uh oh!

thefourtheye commented Jan 4, 2017

Uh oh!

jasnell commented Jan 6, 2017

Uh oh!

jasnell commented Mar 8, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

sivaprs commented Dec 30, 2016 •

edited

Loading