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

url: name anonymous functions in url #9225

Closed
wants to merge 1 commit into from
Closed

url: name anonymous functions in url #9225

wants to merge 1 commit into from

Conversation

pvsousalima
Copy link
Contributor

@pvsousalima pvsousalima commented Oct 21, 2016 •
edited
Loading

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

url

Description of change

url: name anonymous functions in url

name anonymous functions in url 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

@pvsousalima
url: name anonymous functions in url
a285148 
name anonymous functions in url 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
@nodejs-github-bot nodejs-github-bot added the url Issues and PRs related to the legacy built-in url module. label Oct 21, 2016
@jasnell
Copy link
Member

jasnell commented Oct 24, 2016

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

@pvsousalima pvsousalima mentioned this pull request Oct 24, 2016
Closed
2 tasks
@fhinkel
Copy link
Member

fhinkel commented Mar 26, 2017

Thanks. Sorry for taking so long, this PR must have gotten overlooked. Landed in c1dee6a

@fhinkel fhinkel closed this Mar 26, 2017
fhinkel pushed a commit that referenced this pull request Mar 26, 2017
@pvsousalima @fhinkel
url: name anonymous functions in url
c1dee6a 
Name anonymous functions in url.js.

PR-URL: #9225
Ref: #8913
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@lpinca
Copy link
Member

lpinca commented Mar 26, 2017 •
edited
Loading

I think this didn't land before because functions names were already inferred from Url.prototype.

MylesBorins pushed a commit that referenced this pull request Mar 28, 2017
@pvsousalima @MylesBorins
url: name anonymous functions in url
126dcb7 
Name anonymous functions in url.js.

PR-URL: #9225
Ref: #8913
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 28, 2017
Merged
@italoacasas italoacasas mentioned this pull request Apr 10, 2017
Merged
2 tasks
@MylesBorins MylesBorins added the baking-for-lts PRs that need to wait before landing in a LTS release. label May 8, 2017
@MylesBorins MylesBorins added dont-land-on-v6.x and removed baking-for-lts PRs that need to wait before landing in a LTS release. lts-watch-v6.x labels Nov 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

@fhinkel fhinkel fhinkel approved these changes

Assignees
No one assigned
Labels
url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@pvsousalima @jasnell @fhinkel @lpinca @targos @MylesBorins @nodejs-github-bot