Skip to content

src: integrate URL::href() and use in inspector#35912

Closed
watilde wants to merge 1 commit intonodejs:masterfrom
watilde:url-href
Closed

src: integrate URL::href() and use in inspector#35912
watilde wants to merge 1 commit intonodejs:masterfrom
watilde:url-href

Conversation

@watilde
Copy link
Member

@watilde watilde commented Nov 1, 2020 •
edited
Loading

To have the same logic of URL serialization in different modules, I implemented SerializeURL to generate node::url::URL::href() which is mentioned in inspector_agent.cc as ToDo. Migrating lib's side href into C++ would be the next step if there is no blocker.

Refs: #22610

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

@watilde watilde added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Nov 1, 2020
@watilde watilde requested a review from alexkozy November 1, 2020 18:14
@watilde watilde force-pushed the url-href branch 3 times, most recently from dfebd1f to 0e37d9e Compare November 2, 2020 01:37
@watilde watilde added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 2, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 2, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 2, 2020

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

@watilde
Copy link
Member Author

watilde commented Nov 2, 2020

Let me cc @nodejs/url to request your review

Trott
Trott reviewed Nov 2, 2020
View reviewed changes
@watilde
Copy link
Member Author

watilde commented Nov 21, 2020

Landed in e40ed28

@watilde watilde closed this Nov 21, 2020
watilde added a commit that referenced this pull request Nov 21, 2020
@watilde
src: integrate URL::href() and use in inspector
e40ed28 
PR-URL: #35912
Refs: #22610
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@watilde watilde deleted the url-href branch November 21, 2020 15:12
codebytere pushed a commit that referenced this pull request Nov 22, 2020
@watilde @codebytere
src: integrate URL::href() and use in inspector
6d1b1c7 
PR-URL: #35912
Refs: #22610
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@codebytere codebytere mentioned this pull request Nov 22, 2020
Merged
@richardlau richardlau mentioned this pull request Mar 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

@alexkozy alexkozy Awaiting requested review from alexkozy

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. 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.

5 participants

@watilde @nodejs-github-bot @jasnell @Trott @targos