src: replace anonymous closure functions with arrow functions by tpanthera · Pull Request #24443 · nodejs/node

tpanthera · 2018-11-17T13:17:06Z

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

cjihrig

LGTM with the README.md change backed out.

refack · 2018-11-17T21:22:56Z

Hello @tpanthera and welcome, and thank you for your contribution 🥇
If you are not familiar with our review and landing process, it's covered in CONTRIBUTING.md

P.S. If you have any questions you can also feel free to contact me directly.

P.P.S. I rebase your on branch to exclude an unrelated commit, su be sure to git pull before you continue your work.

refack · 2018-11-17T21:23:16Z

Please 👍 to fast-track

richardlau · 2018-11-17T22:11:25Z

This appears to make the test fail:

https://travis-ci.com/nodejs/node/jobs/159298879#L9043-L9064

=== release test-http-localaddress ===
Path: parallel/test-http-localaddress
(node:69520) internal/test/binding: These APIs are exposed only for testing and are not tracked by any versioning system or deprecation process.
/home/travis/build/nodejs/node/test/parallel/test-http-localaddress.js:44
port: this.address().port,
^
TypeError: this.address is not a function
at Server.server.listen (/home/travis/build/nodejs/node/test/parallel/test-http-localaddress.js:44:32)
at Object.onceWrapper (events.js:273:13)
at Server.emit (events.js:182:13)
at emitListeningNT (net.js:1320:10)
at internalTickCallback (internal/process/next_tick.js:72:19)
at process._tickCallback (internal/process/next_tick.js:47:5)
at Function.Module.runMain (internal/modules/cjs/loader.js:778:11)
at startup (internal/bootstrap/node.js:300:19)
at bootstrapNodeJSCore (internal/bootstrap/node.js:826:3)
Command: out/Release/node --expose-internals /home/travis/build/nodejs/node/test/parallel/test-http-localaddress.js
[06:09|% 100|+ 2430|- 1]: Done

Trott

Failing test needs to be fixed.

gireeshpunathil · 2018-11-21T12:28:11Z

ping @tpanthera

Co-Authored-By: tpanthera <bisht11390@gmail.com>

gireeshpunathil · 2018-11-23T11:51:32Z

+server.listen(0, '127.0.0.1', () => {
 const options = { host: 'localhost',
- port: this.address().port,
+port: server.address().port,


@tpanthera - once more issue here - you put one extra space in line 44 before the code. Align it to the previous line, and I guess we are all good here.

Thanks for correcting, committed the revised code.

* Revised code alignment.

gireeshpunathil · 2018-11-23T12:24:29Z

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

gireeshpunathil · 2018-11-23T12:24:52Z

@Trott - PTAL.

test fixed

PR-URL: nodejs#24443 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

Trott · 2018-11-28T00:22:20Z

Landed in 3ec8576.

Thanks for the contribution! 🎉

(If you're interested in other possible contributions to Node.js but don't have a good idea of where to start looking, some ideas are posted at https://www.nodetodo.org/next-steps/.)

PR-URL: #24443 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

PR-URL: nodejs#24443 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

PR-URL: #24443 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Nov 17, 2018

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

cjihrig approved these changes Nov 17, 2018
View reviewed changes

refack changed the title ~~Qualitycodeimprovements~~src: replace anonymous closure functions with arrow functionsNov 17, 2018

src: replace anonymous closure functions with arrow functions
1538ba9

refack force-pushed the qualitycodeimprovements branch from 804dd81 to 1538ba9Compare November 17, 2018 21:21

refack approved these changes Nov 17, 2018
View reviewed changes

refack added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 17, 2018

refack removed the fast-track PRs that do not need to wait for 48 hours to land. label Nov 17, 2018

refack reviewed Nov 17, 2018
View reviewed changes

Comment threadtest/parallel/test-http-localaddress.js Outdated

trivikr approved these changes Nov 18, 2018
View reviewed changes

jasnell approved these changes Nov 19, 2018
View reviewed changes

sagirk approved these changes Nov 19, 2018
View reviewed changes

Trott previously requested changes Nov 21, 2018
View reviewed changes

Update test/parallel/test-http-localaddress.js
c519542
Co-Authored-By: tpanthera <bisht11390@gmail.com>

gireeshpunathil reviewed Nov 23, 2018
View reviewed changes

Update test-http-localaddress.js
f395b09
* Revised code alignment.

gireeshpunathil approved these changes Nov 23, 2018
View reviewed changes

trivikr approved these changes Nov 23, 2018
View reviewed changes

Trott closed this Nov 28, 2018

BridgeAR mentioned this pull request Dec 5, 2018
v11.4.0 proposal #24854
Merged
4 tasks

BethGriggs mentioned this pull request Feb 12, 2019
v10.15.3 proposal #26063
Merged

Navigation Menu

Uh oh!

Uh oh!

src: replace anonymous closure functions with arrow functions - #24443

src: replace anonymous closure functions with arrow functions#24443
tpanthera wants to merge 3 commits into
nodejs:masterfrom
tpanthera:qualitycodeimprovements

tpanthera commented Nov 17, 2018•
edited
Loading

Uh oh!

cjihrig left a comment

Uh oh!

refack commented Nov 17, 2018

Uh oh!

refack commented Nov 17, 2018

Uh oh!

richardlau commented Nov 17, 2018

Uh oh!

Uh oh!

Trott left a comment

Uh oh!

gireeshpunathil commented Nov 21, 2018

Uh oh!

gireeshpunathilNov 23, 2018

Uh oh!

tpantheraNov 23, 2018

Uh oh!

gireeshpunathil commented Nov 23, 2018

Uh oh!

gireeshpunathil commented Nov 23, 2018

Uh oh!

Trott commented Nov 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Checklist

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

refack commented Nov 17, 2018

Uh oh!

refack commented Nov 17, 2018

Uh oh!

richardlau commented Nov 17, 2018

Uh oh!

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

gireeshpunathil commented Nov 21, 2018

Uh oh!

gireeshpunathilNov 23, 2018

Choose a reason for hiding this comment

Uh oh!

tpantheraNov 23, 2018

Choose a reason for hiding this comment

Uh oh!

gireeshpunathil commented Nov 23, 2018

Uh oh!

gireeshpunathil commented Nov 23, 2018

Uh oh!

Trott commented Nov 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

tpanthera commented Nov 17, 2018•
edited
Loading