Skip to content

Conversation

@annatangzhao
Copy link
Contributor

@annatangzhaoannatangzhao commented Oct 12, 2018

Part of Code and Learn workshop at Node + JS Interactive!

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

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Oct 12, 2018
@targostargos added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 12, 2018
consthost='127.0.0.1';
constsock=dgram.createSocket('udp4');

assert.throws(function(){
Copy link
Member

Choose a reason for hiding this comment

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

Since the function contains only one statement, the curly brackets are not required here and other places in the code.
That means, () =>{sock.send()} can be () => sock.send()

Copy link
Member

@TrottTrottOct 14, 2018

Choose a reason for hiding this comment

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

In my opinion, the brackets should not be removed in this situation. They should only be removed if the arrow function is explicitly returning a value (in my opinion).

Copy link
Member

@TrottTrottOct 14, 2018

Choose a reason for hiding this comment

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

In other words:

() =>{return foo()} should be changed to () => foo(); but () =>{foo()} should be left as-is, in my opinion. Otherwise, someone reading the code is left wondering if the fact that the arrow function is returning a value is a result of the author wanting to omit the brackets for the sake of omitting the brackets or if it's because the function is meant to return a value.

@TrottTrott mentioned this pull request Oct 14, 2018
@Trott
Copy link
Member

@Trott
Copy link
Member

Landed in 6783eed

@TrottTrott closed this Oct 15, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Oct 15, 2018
PR-URL: nodejs#23483 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
jasnell pushed a commit that referenced this pull request Oct 17, 2018
PR-URL: #23483 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
addaleax pushed a commit that referenced this pull request Oct 20, 2018
PR-URL: #23483 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 30, 2018
PR-URL: #23483 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
@codebyterecodebytere mentioned this pull request Nov 27, 2018
rvagg pushed a commit that referenced this pull request Nov 28, 2018
PR-URL: #23483 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
PR-URL: #23483 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
@codebyterecodebytere mentioned this pull request Nov 29, 2018
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.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@annatangzhao@Trott@jasnell@codebytere@cjihrig@gireeshpunathil@trivikr@targos@nodejs-github-bot