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.

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

https: name anonymous functions in https #9217

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@pvsousalima
Contributor

pvsousalima commented Oct 21, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

https

Description of change

https: name anonymous functions in https

name anonymous functions in https module
the changes are related to commits ef030da and accf410
which are naming anonymous functions to standardize the code
and help debugging, also with Ref: #8913

@lpinca

lpinca approved these changes Oct 24, 2016

@targos

targos approved these changes Oct 24, 2016

@targos

This comment has been minimized.

Show comment
Hide comment

@pvsousalima pvsousalima referenced this pull request Oct 24, 2016

Closed

os: name every function OS module #9250

2 of 2 tasks complete
@lpinca

This comment has been minimized.

Show comment
Hide comment
@lpinca

lpinca Oct 25, 2016

Member

I think that it is fine (but not necessary?) to also name prototype functions. Still LGTM.

Member

lpinca commented Oct 25, 2016

I think that it is fine (but not necessary?) to also name prototype functions. Still LGTM.

pvsousalima added some commits Oct 21, 2016

https: name anonymous functions in https
related to commit ef030da
which is naming anonymous functions to standardize the code
https: name anonymous functions in https
related to commit ef030da
which is naming anonymous functions to standardize the code
https: name anonymous functions
As stated in #9252 anonymous callback functions should
be named, but not functions that are assigned to a property
on a variable because the name is inferred from the assignment
@italoacasas

This comment has been minimized.

Show comment
Hide comment
@italoacasas

italoacasas Nov 25, 2016

Member

Landed in: 1f45d7a

Thanks for the contribution.

Member

italoacasas commented Nov 25, 2016

Landed in: 1f45d7a

Thanks for the contribution.

italoacasas added a commit that referenced this pull request Nov 25, 2016

https: name anonymous functions in https
Naming anonymous function in the https module

PR-URL: #9217
Ref: #8913
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

addaleax added a commit that referenced this pull request Dec 5, 2016

https: name anonymous functions in https
Naming anonymous function in the https module

PR-URL: #9217
Ref: #8913
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

@Fishrock123 Fishrock123 referenced this pull request Dec 5, 2016

Merged

v7.2.1 proposal #10127

2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment