Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

net: named anonymous functions #21062

Closed
wants to merge 1 commit into from
Closed

net: named anonymous functions #21062

wants to merge 1 commit into from

Conversation

ErnestoSalazar
Copy link
Contributor

Named anonymous functions #8913

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

@nodejs-github-bot nodejs-github-bot added the net Issues and PRs related to the net subsystem. label May 31, 2018
trivikr
trivikr reviewed May 31, 2018
View reviewed changes
lib/net.js
@@ -338,7 +338,7 @@ function shutdownSocket(self, callback) {

// the user has called .end(), and all the bytes have been
// sent out to the other side.
Socket.prototype._final = function(cb) {
Copy link
Member

Choose a reason for hiding this comment

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

@ErnestoSalazar Congrats on your first PR in Node.js!
However, everything assigned to prototype should be good (comment) and not required to be named.

Copy link
Contributor Author

@ErnestoSalazar ErnestoSalazar May 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@trivikr So, i see that other files also follows that same convention like https.js, thanks for your fast reply :D

@Trott
Copy link
Member

Trott commented Jun 1, 2018

Hi again, @ErnestoSalazar! I think there's nothing to be done on net.js for naming anonymous functions at this point. I'm going to close this, but comment or re-open if I'm mistaken! And thanks for the pull request!

@Trott Trott closed this Jun 1, 2018
@snyk-bot snyk-bot mentioned this pull request Mar 18, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivikr trivikr trivikr left review comments

Assignees
No one assigned
Labels
net Issues and PRs related to the net subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ErnestoSalazar @Trott @trivikr @nodejs-github-bot