FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: encode 0x27 (') for special URLs by TimothyGu · Pull Request #22022 · nodejs/node · GitHub

/ node Public

src: encode 0x27 (') for special URLs - #22022

Closed
TimothyGu wants to merge 1 commit into
nodejs:masterfrom
TimothyGu:url-updae
ConversationCommits (1)ChecksFiles changed
Closed

src: encode 0x27 (') for special URLs#22022
TimothyGu wants to merge 1 commit into
nodejs:masterfrom
TimothyGu:url-updae

Conversation

TimothyGu commented Jul 30, 2018

Copy link
Copy Markdown
Member

Refs: whatwg/url@6ef17eb

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

nodejs-github-bot commented Jul 30, 2018

Copy link
Copy Markdown
Collaborator

@TimothyGu build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/377/pipeline

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Jul 30, 2018
mscdex reviewed Jul 30, 2018
View reviewed changes
Comment thread test/fixtures/url-tests.js
/* The following tests are copied from WPT. Modifications to them should be
upstreamed first. Refs:
https://github.com/w3c/web-platform-tests/blob/88b75886e/url/urltestdata.json
https://github.com/web-platform-tests/wpt/blob/ba4921d054808a44c2966507c19861ca729a76a2/url/resources/urltestdata.json

mscdex Jul 30, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Minor nit: perhaps we could make this URL at least a little shorter by reducing the length of the commit/tree hash?

TimothyGu Aug 18, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Done.

maclover7 commented Aug 3, 2018

Copy link
Copy Markdown
Contributor

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

maclover7 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 9, 2018

jasnell commented Aug 12, 2018

Copy link
Copy Markdown
Member

New CI (since the previous results are not accessible): https://ci.nodejs.org/job/node-test-pull-request/16382/

jasnell commented Aug 12, 2018

Copy link
Copy Markdown
Member

Running linux-containered CI again: https://ci.nodejs.org/job/node-test-commit-linux-containered/6196/

jasnell commented Aug 12, 2018

Copy link
Copy Markdown
Member

@nodejs/build ... linux-containered buildbot is having some issue here:

12:55:20 + git rebase --committer-date-is-author-date 01a160a05de56b8d8037250d28ce1ba4304a036c
12:55:20 Cannot rebase: You have unstaged changes.
12:55:20 Please commit or stash them.

mmarchini commented Aug 12, 2018

Copy link
Copy Markdown
Contributor

@nodejs/build ... linux-containered buildbot is having some issue here

On it. Seems to be some issue with EOL characters on deps/v8/third_party/jinja2/LICENSE.

mmarchini commented Aug 12, 2018 •
edited by TimothyGu
Loading

Copy link
Copy Markdown
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/16388/ ❌
CI: https://ci.nodejs.org/job/node-test-pull-request/16398/ ❌
CI: https://ci.nodejs.org/job/node-test-pull-request/16423/ ✅

TimothyGu commented Aug 18, 2018

Copy link
Copy Markdown
Member Author

Landed in c7788fb.

TimothyGu closed this Aug 18, 2018
TimothyGu deleted the url-updae branch August 18, 2018 19:18
TimothyGu added a commit that referenced this pull request Aug 18, 2018
src: encode 0x27 (') for special URLs
c7788fb 
Refs: whatwg/url@6ef17eb
PR-URL: #22022
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Aug 19, 2018
src: encode 0x27 (') for special URLs
fb25130 
Refs: whatwg/url@6ef17eb
PR-URL: #22022
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Sep 3, 2018
src: encode 0x27 (') for special URLs
c207865 
Refs: whatwg/url@6ef17eb
PR-URL: #22022
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos mentioned this pull request Sep 5, 2018
Merged
This was referenced Sep 6, 2018
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
lpinca lpinca approved these changes
cjihrig cjihrig approved these changes
BridgeAR BridgeAR approved these changes
+2 more reviewers
mscdex mscdex left review comments
targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

Back | FazBrowse Home | New Git URL