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

doc: change urls directly from 'http' to 'https' #29422

Closed
wants to merge 1 commit into from
Closed

doc: change urls directly from 'http' to 'https' #29422

wants to merge 1 commit into from

Conversation

SEWeiTung
Copy link

@SEWeiTung SEWeiTung commented Sep 3, 2019 •
edited
Loading

There're some URLs with old links, change them together from 'http' to
'https'.

Notice:

  1. Since files of CHANGELOG may be generated through tools, I don't
    intend to change them together as the history track.
  2. All the files in the 'deps' are of 3-rd parties, they will be
    overwritten for the next update, so avoid modifications for them.
  • 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 build Issues and PRs related to build files or the CI. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. doc Issues and PRs related to the documentations. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. libuv Issues and PRs related to the libuv dependency or the uv binding. npm Issues and PRs related to the npm client dependency or the npm registry. openssl Issues and PRs related to the OpenSSL dependency. labels Sep 3, 2019
trivikr
trivikr reviewed Sep 3, 2019
View reviewed changes
deps/uv/README.md Outdated Show resolved Hide resolved
lpinca
lpinca approved these changes Sep 3, 2019
View reviewed changes
Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM with @mscdex's comment addressed

@SEWeiTung SEWeiTung changed the title doc: change urls directly or from 'http' to 'https' doc: change urls directly from 'http' to 'https' Sep 3, 2019
bnoordhuis
bnoordhuis requested changes Sep 4, 2019
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

Everything in deps/ are third-party dependencies. Don't change those, such changes only get lost on the next update.

doc: change urls directly from 'http' to 'https'
3f5216a 
There're some URLs with old links, change them together from 'http' to
'https'.

Notice:
1. Since files of CHANGELOG may be generated through tools, I don't
intend to change them together as the history track.
2. All the files in the 'deps' are of 3-rd parties, they will be
overwritten for the next update, so avoid modifications for them.
@SEWeiTung
Copy link
Author

@bnoordhuis. Fixed.
Do they come from the generation tools or something else? Can I modify that from 'http' to 'https'?

@bnoordhuis
Copy link
Member

Do they come from the generation tools or something else?

They're all third-party sources that we vendor in.

@Trott
Copy link
Member

Trott commented Sep 6, 2019

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3884/

@Trott
Copy link
Member

Trott commented Sep 6, 2019

Landed in c6b7e74

@Trott Trott closed this Sep 6, 2019
Trott pushed a commit that referenced this pull request Sep 6, 2019
@Trott
doc: change urls directly from 'http' to 'https'
c6b7e74 
There're some URLs with old links, change them together from 'http' to
'https'.

Notice:
1. Since files of CHANGELOG may be generated through tools, I don't
intend to change them together as the history track.
2. All the files in the 'deps' are of 3-rd parties, they will be
overwritten for the next update, so avoid modifications for them.

PR-URL: #29422
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@SEWeiTung SEWeiTung deleted the FixByAddings branch September 6, 2019 05:11
targos pushed a commit that referenced this pull request Sep 20, 2019
@targos
doc: change urls directly from 'http' to 'https'
844b45b 
There're some URLs with old links, change them together from 'http' to
'https'.

Notice:
1. Since files of CHANGELOG may be generated through tools, I don't
intend to change them together as the history track.
2. All the files in the 'deps' are of 3-rd parties, they will be
overwritten for the next update, so avoid modifications for them.

PR-URL: #29422
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@BridgeAR BridgeAR mentioned this pull request Sep 24, 2019
Merged
This was referenced Sep 25, 2019
Closed
Closed
Closed
This was referenced Sep 26, 2019
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mscdex mscdex mscdex left review comments

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@trivikr trivikr trivikr approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. doc Issues and PRs related to the documentations. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. libuv Issues and PRs related to the libuv dependency or the uv binding. npm Issues and PRs related to the npm client dependency or the npm registry. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@SEWeiTung @bnoordhuis @Trott @mscdex @jasnell @lpinca @trivikr @nodejs-github-bot