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.

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

url: fix url.format with ipv6 hostname #36665

Closed
wants to merge 1 commit into from

Conversation

Lxxyx
Copy link
Member

@Lxxyx Lxxyx commented Dec 28, 2020

Fixes: #36654

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the url Issues and PRs related to the legacy built-in url module. label Dec 28, 2020
@ronag ronag added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Dec 28, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 28, 2020
@nodejs-github-bot
Copy link
Collaborator

Copy link
Contributor

@yashLadha yashLadha left a comment

Choose a reason for hiding this comment

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

Minor changes to use primordials instead of built-in, else changes LGTM.

lib/url.js Outdated Show resolved Hide resolved
@yashLadha yashLadha added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 30, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 30, 2020
@github-actions
Copy link
Contributor

Commit Queue failed
- Loading data for nodejs/node/pull/36665
✔  Done loading data for nodejs/node/pull/36665
----------------------------------- PR info ------------------------------------
Title      url: fix url.format with ipv6 hostname (#36665)
Author     Lxxyx  (@Lxxyx)
Branch     Lxxyx:fix-url-ipv6-formats -> nodejs:master
Labels     author ready, url
Commits    1
 - url: fix url.format with ipv6 hostname
Committers 1
 - ZiJian Liu 
PR-URL: https://github.com/nodejs/node/pull/36665
Fixes: https://github.com/nodejs/node/issues/36654
Reviewed-By: Luigi Pinca 
Reviewed-By: Robert Nagy 
Reviewed-By: Rich Trott 
Reviewed-By: Daijiro Wachi 
Reviewed-By: Yash Ladha 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/36665
Fixes: https://github.com/nodejs/node/issues/36654
Reviewed-By: Luigi Pinca 
Reviewed-By: Robert Nagy 
Reviewed-By: Rich Trott 
Reviewed-By: Daijiro Wachi 
Reviewed-By: Yash Ladha 
--------------------------------------------------------------------------------
   ✔  Last GitHub Actions successful
   ℹ  Last Full PR CI on 2020-12-28T18:44:50Z: https://ci.nodejs.org/job/node-test-pull-request/35129/
   ⚠  Commits were pushed after the last Full PR CI run:
   ⚠  - url: fix url.format with ipv6 hostname
- Querying data for job/node-test-pull-request/35129/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
   ℹ  This PR was created on Mon, 28 Dec 2020 15:47:53 GMT
   ✔  Approvals: 5
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/36665#pullrequestreview-559235401
   ✔  - Robert Nagy (@ronag): https://github.com/nodejs/node/pull/36665#pullrequestreview-559254174
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/36665#pullrequestreview-559361009
   ✔  - Daijiro Wachi (@watilde): https://github.com/nodejs/node/pull/36665#pullrequestreview-559472531
   ✔  - Yash Ladha (@yashLadha): https://github.com/nodejs/node/pull/36665#pullrequestreview-559779284
   ✖  This PR needs to wait 51 more minutes to land
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/452857792

@github-actions github-actions bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Dec 30, 2020
@yashLadha yashLadha added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 30, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 30, 2020
@github-actions
Copy link
Contributor

Landed in 37acaf6...1b7ac0c

@github-actions github-actions bot closed this Dec 30, 2020
nodejs-github-bot pushed a commit that referenced this pull request Dec 30, 2020
Fixes: #36654

PR-URL: #36665
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Yash Ladha <yash@yashladha.in>
@Lxxyx Lxxyx deleted the fix-url-ipv6-formats branch December 31, 2020 03:51
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
Fixes: #36654

PR-URL: #36665
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Yash Ladha <yash@yashladha.in>
@danielleadams danielleadams mentioned this pull request Jan 12, 2021
targos pushed a commit that referenced this pull request May 1, 2021
Fixes: #36654

PR-URL: #36665
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Yash Ladha <yash@yashladha.in>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The deprecated url.format formats ipv6 localhost address wrongly
7 participants