Change callback to ES6 style by jamesgeorge007 · Pull Request #24513 · nodejs/node

jamesgeorge007 · 2018-11-20T08:02:14Z

Converted the es5 functions present within the test/pummel/test-net-pause.js to arrow (es6) functions which is more concise.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

test/pummel/test-net-pause.js

jamesgeorge007 · 2018-11-21T18:49:36Z

@Trott What do you think now?

gireeshpunathil · 2018-11-22T08:38:56Z

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

gireeshpunathil · 2018-11-22T11:20:50Z

earlier CI run was aborted. new CI: https://ci.nodejs.org/job/node-test-pull-request/18879/

addressed

addaleax · 2018-11-23T02:14:11Z

@jamesgeorge007 If you could rebase out the merge commit here, that could help – our CI doesn’t play well with those.

CI (rebasing disabled): https://ci.nodejs.org/job/node-test-pull-request/18890/

jamesgeorge007 · 2018-11-23T16:17:56Z

@addaleax My working tree became dirty. Hence, I had to close the PR and reopen it again.
Hope everythings fine now 👍

addaleax · 2018-11-23T21:17:14Z

@jamesgeorge007 Hm … I think the linter failure from https://travis-ci.com/nodejs/node/jobs/160538871 is real:

$ make lint
make[1]: Entering directory `/home/travis/build/nodejs/node'
Running JS linter...
/home/travis/build/nodejs/node/test/pummel/test-net-pause.js
  91:4  error  Newline required at end of file but not found  eol-last
✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

addaleax · 2018-11-24T02:53:48Z

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

Trott · 2018-11-24T06:06:32Z

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

gireeshpunathil · 2018-11-24T15:12:28Z

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

test/pummel/test-net-pause.js

add newline at EOF 👍 Add EOF 👍 fix 👍 fix 👍 fix 👍

gireeshpunathil · 2018-11-25T12:33:03Z

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

gireeshpunathil · 2018-11-25T14:17:50Z

landed as afab340

thank you @jamesgeorge007 for the contribution! Wish you great success with continued contribution to this project, if you are further interested please have a look at https://www.nodetodo.org/next-steps

PR-URL: #24513 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

jamesgeorge007 · 2018-11-25T14:25:41Z

👍

gireeshpunathil · 2018-11-25T15:02:59Z

@gireeshpunathil What was the reason behind closing this PR instead of merging? Just curious

@jamesgeorge007 - don't worry; your code is landed as afab340 into the repo, and you have become a contributor in the project.

PR-URL: #24513 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

PR-URL: nodejs#24513 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

PR-URL: #24513 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

nodejs-github-bot added the test Issues and PRs related to the tests. label Nov 20, 2018

BridgeAR approved these changes Nov 20, 2018

View reviewed changes

cjihrig approved these changes Nov 20, 2018

View reviewed changes

jasnell approved these changes Nov 21, 2018

View reviewed changes

gireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 21, 2018

Trott previously requested changes Nov 21, 2018

View reviewed changes

test/pummel/test-net-pause.js Outdated Show resolved Hide resolved

gireeshpunathil approved these changes Nov 22, 2018

View reviewed changes

jamesgeorge007 force-pushed the patch-1 branch from da24ccb to 8e3c6e6 Compare November 23, 2018 15:56

jamesgeorge007 closed this Nov 23, 2018

jamesgeorge007 force-pushed the patch-1 branch from 8e3c6e6 to b32c5f0 Compare November 23, 2018 16:10

jamesgeorge007 reopened this Nov 23, 2018

addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 24, 2018

jamesgeorge007 force-pushed the patch-1 branch 2 times, most recently from 2947c85 to d32d64c Compare November 24, 2018 03:07

trivikr approved these changes Nov 24, 2018

View reviewed changes

jamesgeorge007 force-pushed the patch-1 branch from 2aa26f9 to fbce410 Compare November 24, 2018 11:20

jamesgeorge007 force-pushed the patch-1 branch 3 times, most recently from 17a9ef5 to b62fe77 Compare November 24, 2018 16:53

Trott reviewed Nov 25, 2018

View reviewed changes

test/pummel/test-net-pause.js Outdated Show resolved Hide resolved

Trott removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 25, 2018

jamesgeorge007 force-pushed the patch-1 branch 3 times, most recently from cdb949f to dee06f8 Compare November 25, 2018 09:35

test: convert callback to arrow function

b210dc2

add newline at EOF 👍 Add EOF 👍 fix 👍 fix 👍 fix 👍

jamesgeorge007 force-pushed the patch-1 branch from dee06f8 to b210dc2 Compare November 25, 2018 10:19

jamesgeorge007 changed the title ~~Convert functions to es6 style~~ Change callback to ES6 style Nov 25, 2018

gireeshpunathil closed this Nov 25, 2018

refack added the landed label Nov 25, 2018

BridgeAR mentioned this pull request Dec 5, 2018

v11.4.0 proposal #24854

Merged

4 tasks

BethGriggs added the land-on-v10.x label Feb 11, 2019

BethGriggs mentioned this pull request Feb 12, 2019

v10.15.3 proposal #26063

Merged

Uh oh!

Conversation

jamesgeorge007 commented Nov 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Uh oh!

jamesgeorge007 commented Nov 21, 2018

Uh oh!

gireeshpunathil commented Nov 22, 2018

Uh oh!

gireeshpunathil commented Nov 22, 2018

Uh oh!

addaleax commented Nov 23, 2018

Uh oh!

jamesgeorge007 commented Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

addaleax commented Nov 23, 2018

Uh oh!

addaleax commented Nov 24, 2018

Uh oh!

Trott commented Nov 24, 2018

Uh oh!

gireeshpunathil commented Nov 24, 2018

Uh oh!

Uh oh!

gireeshpunathil commented Nov 25, 2018

Uh oh!

gireeshpunathil commented Nov 25, 2018

Uh oh!

jamesgeorge007 commented Nov 25, 2018

Uh oh!

gireeshpunathil commented Nov 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

jamesgeorge007 commented Nov 20, 2018 •

edited

Loading

jamesgeorge007 commented Nov 23, 2018 •

edited

Loading