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

inspector: update faviconUrl #29562

Closed
wants to merge 2 commits into from
Closed

inspector: update faviconUrl #29562

wants to merge 2 commits into from

Conversation

dokugo
Copy link
Contributor

@dokugo dokugo commented Sep 15, 2019

Seeing no icon for the remote target at chrome://inspect/#devices when debugging via Chrome DevTools with --inspect switch. GET request for the icon is returning 404 status code because of incorrect request URL: https://nodejs.org/static/favicon.ico.
Changed faviconUrl at src/inspector_socket_server.cc to https://nodejs.org/static/images/favicons/favicon.ico. The reason for all this is icon files being moved in nodejs/nodejs.org@ce04045 commit.

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

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol labels Sep 15, 2019
@devnexen
Copy link
Contributor

Thanks for your contribution. Fine by me just little detail from lint
src/inspector_socket_server.cc:346: Lines should be <= 80 characters long [whitespace/line_length] [2]

@ZYSzys
Copy link
Member

ZYSzys commented Sep 15, 2019

Ping @dokugo , could you please fix the linter so that we can land it ?

@dokugo
Copy link
Contributor Author

dokugo commented Sep 15, 2019

Fixed the linter issue. I should have run make lint initially but it slipped off my mind.

@nodejs-github-bot
Copy link
Collaborator

@danbev danbev added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 4, 2019
danbev pushed a commit that referenced this pull request Oct 4, 2019
PR-URL: #29562
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@danbev
Copy link
Contributor

danbev commented Oct 4, 2019

Landed in 24d43ca. 🎉

@danbev danbev closed this Oct 4, 2019
BridgeAR pushed a commit that referenced this pull request Oct 9, 2019
PR-URL: #29562
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR BridgeAR mentioned this pull request Oct 10, 2019
@dokugo dokugo deleted the update-favicon-url branch November 9, 2019 01:45
@dokugo dokugo restored the update-favicon-url branch November 9, 2019 01:45
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. c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants