Skip to content

Use https instead of http #1476

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

Closed
wants to merge 1 commit into from
Closed

Conversation

komainu8
Copy link
Contributor

@komainu8 komainu8 commented Aug 1, 2019

Dear, developers.
I've fixed an issue #1459.

@exoego
Copy link
Contributor

exoego commented Aug 1, 2019

There is already your pull request #1471 addressing the issue #1459
What is a reason to separate PRs?
I think an ordinal issue (not meta issue) should be addressed in single PR for most cases, so that reviewers close the issue after PR merged.

@komainu8
Copy link
Contributor Author

komainu8 commented Aug 2, 2019

@exoego Thank you for your comment.
I agree with you. I think I should put those pull request in one.

However, files that should modify the same as this pull request is many.

I think that is difficult to review all modify at once.
Therefore, I split one pull request by the one file.

If we should put all modify in one is better, I'll do so.

I'm sorry, I'm bad at English.
Is what I'm saying getting through to you properly?

@exoego
Copy link
Contributor

exoego commented Aug 2, 2019

Don't mind, it is clear what you are saying.

So, I guess you are afraid of broken links (404) that reviewers should take care, if replace all http:// into http:// at once ?
If so, please do not mind !!
There is a link checker in this repo, and it verifies that all links are valid (with some exception for oracle.com) on pull requests.
If 404 found, you can find easily which links need some tweaks (e.g.: there is no https://... for the domain).

So I suggest to

  1. Replace all http:// to https:// and add all into single commit.
  2. Push and watch link checker result.
  3. If there are 404, fix them and create a new commit. So reviewers can spot only URLs that carefull review are required.

@komainu8
Copy link
Contributor Author

komainu8 commented Aug 2, 2019

Oh! That's a great mechanism.
Yes. I was afraid of broken links (404) that reviewers should take care of.

Thank you for your suggestion.
I'll accept your suggestion.

First, I'll close this PR.
Second, I replace all http:// to https:// and add all modify into #1471.
Third, I check links and repair broken links.

@komainu8 komainu8 closed this Aug 2, 2019
@komainu8 komainu8 deleted the use_https_learn branch August 2, 2019 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants