Skip to content

HTTPS links in documentation #5965

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 2 commits into from

Conversation

himanshu007-creator
Copy link

Resolved #5163

@jbj
Copy link
Contributor

jbj commented May 31, 2021

I think some of these changes could be valuable. Before any of us starts reviewing in detail, I'd like to check with you, @himanshu007-creator, whether you're prepared to rework the PR in response to review comments about removing unrelated changes or changes to files that should remain static.

@himanshu007-creator
Copy link
Author

Yes, I can am able to rework on this PR

@esbena
Copy link
Contributor

esbena commented May 31, 2021

This PR touches 15000 lines, across 1200 files. Which is too much to review manually. Could you perhaps explain how you selected the files to change, and what you changed in them? I suppose you used some kind of script.


By the way, a quick look at the diff reveals the following example changes that does not match the PR description "HTTPS links in documentation":

@himanshu007-creator
Copy link
Author

Yeah, i just searched for all instances of http: and replaced them with https:

@esbena
Copy link
Contributor

esbena commented Jun 1, 2021

As seen above, that approach is not precise enough. Do you want to refine your replacement strategy?

@himanshu007-creator
Copy link
Author

Yes, I will make the changes as suggested and improve this PR.

@esbena
Copy link
Contributor

esbena commented Jun 1, 2021

Just to make it clear: the three undesired changes that I listed above is not an exhaustive list.
I would prefer to see a description of how you selected the files to change, and what you changed in them, instead of having to spelunk for other undesired changes.

@esbena
Copy link
Contributor

esbena commented May 9, 2022

Closing the PR due to inactivity, but feel free to reopen if you wish to continue work on it.

You need to rerun the script in any case to get past the conflicts.

@esbena esbena closed this May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use HTTPS links in documentation and comments
3 participants