-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
bpo-25910: Link redirections in docs #1933
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
Conversation
|
@CuriousLearner, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @serhiy-storchaka, @ncoghlan and @tiran to be potential reviewers. |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have doubts about most of changes. Links shouldn't contain language or version specific parts or expose implementation details of a site if the original link is working.
Replacing "http" with "https" LGTM. Removing "/en/latest/" LGTM.
Doc/faq/extending.rst
Outdated
| <http://cxx.sourceforge.net/>`_ `Boost | ||
| <http://www.boost.org/libs/python/doc/index.html>`_, or `Weave | ||
| <https://scipy.github.io/devdocs/tutorial/weave.html>`_ are also | ||
| <http://www.boost.org/doc/libs/1_64_0/libs/python/doc/html/index.html>`_, or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The redirection is changed regularly after releasing every new version of Boost. This FAQ is not specific to particular Boost version. The general version of the link should be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Doc/about.rst
Outdated
|
|
||
| .. _reStructuredText: http://docutils.sourceforge.net/rst.html | ||
| .. _Sphinx: http://sphinx-doc.org/ | ||
| .. _Sphinx: http://www.sphinx-doc.org/en/stable/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This replaces language agnostic link with language specific link. If the resource has versions on other languages (or will have in future) and the redirection is based on user's preferences, this replacement is not valid.
www. is optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, indeed you're right about this. I've made the changes
Doc/faq/general.rst
Outdated
| still like to know about all commercial use of Python, of course. | ||
|
|
||
| See `the PSF license page <https://www.python.org/psf/license/>`_ to find further | ||
| See `the PSF license page <https://docs.python.org/3/license.html>`_ to find further |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep the original link.
Doc/faq/gui.rst
Outdated
| ---- | ||
|
|
||
| Python bindings for `the FLTK toolkit <http://www.fltk.org>`_, a simple yet | ||
| Python bindings for `the FLTK toolkit <http://www.fltk.org/index.php>`_, a simple yet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/index.php is an implementation detail. It just means that currently the site is implemented in PHP. After rewiting it on Python that link likely will become not valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that is an issue in that website, not in this docs, isn’t it?
If the goal of the PR is to avoid redirects, and the fltk website currently redirects / to /whatever.implementiation.detail, the change here is correct?
CuriousLearner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @serhiy-storchaka !
I've fixed the issues you mentioned.
Doc/about.rst
Outdated
|
|
||
| .. _reStructuredText: http://docutils.sourceforge.net/rst.html | ||
| .. _Sphinx: http://sphinx-doc.org/ | ||
| .. _Sphinx: http://www.sphinx-doc.org/en/stable/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, indeed you're right about this. I've made the changes
Doc/faq/extending.rst
Outdated
| <http://cxx.sourceforge.net/>`_ `Boost | ||
| <http://www.boost.org/libs/python/doc/index.html>`_, or `Weave | ||
| <https://scipy.github.io/devdocs/tutorial/weave.html>`_ are also | ||
| <http://www.boost.org/doc/libs/1_64_0/libs/python/doc/html/index.html>`_, or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
Please fix all similar issues. |
|
@serhiy-storchaka sure, on it. |
|
@serhiy-storchaka in the |
Doc/distributing/index.rst
Outdated
| other Python users | ||
| * the `Python Packaging Authority | ||
| <https://www.pypa.io/>`__ are the group of | ||
| <https://www.pypa.io/en/latest/>`__ are the group of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@serhiy-storchaka like here. should I remove these kind of changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, remove.
|
If the web-server returns the HTTP code in the range 3XX except 301 (Moved Permanently) or 308 (Permanent Redirect), we should keep the original link. For www.pypa.io the code 302 (Found) is returned. I think it is better to remove all additions of |
|
I've tried to resolve all the issues with redirection of links. However I've a concern: Should we fix these in this PR? |
Not in this PR. If there are reasons for using |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have reviewed a half of the PR. Please remove insignificant changes and changes with HTTP code 302. Make similar changes in the other half.
Doc/about.rst
Outdated
|
|
||
| .. _reStructuredText: http://docutils.sourceforge.net/rst.html | ||
| .. _Sphinx: http://sphinx-doc.org/ | ||
| .. _Sphinx: http://www.sphinx-doc.org |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP/1.1 302 Found
Keep the original link.
Doc/distributing/index.rst
Outdated
| other Python users | ||
| * the `Python Packaging Authority | ||
| <https://www.pypa.io/>`__ are the group of | ||
| <https://www.pypa.io/en/latest/>`__ are the group of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, remove.
| .. seealso:: | ||
|
|
||
| `How to Report Bugs Effectively <http://www.chiark.greenend.org.uk/~sgtatham/bugs.html>`_ | ||
| `How to Report Bugs Effectively <https://www.chiark.greenend.org.uk/~sgtatham/bugs.html>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay.
|
|
||
| .. _setuptools: https://setuptools.readthedocs.io/en/latest/ | ||
| .. _wheel: https://wheel.readthedocs.org | ||
| .. _wheel: https://wheel.readthedocs.io/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay.
Doc/distributing/index.rst
Outdated
| recommended tools`_. | ||
|
|
||
| .. _currently recommended tools: https://packaging.python.org/en/latest/current/#packaging-tool-recommendations | ||
| .. _currently recommended tools: https://packaging.python.org/current/#packaging-tool-recommendations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, it seems like the URL is now https://packaging.python.org/guides/tool-recommendations/#packaging-tool-recommendations ?
Doc/faq/programming.rst
Outdated
| length, whether variable names are well-formed according to your coding | ||
| standard, whether declared interfaces are fully implemented, and more. | ||
| https://docs.pylint.org/ provides a full list of Pylint's features. | ||
| https://pylint.readthedocs.io/ provides a full list of Pylint's features. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP/1.1 302 FOUND
Keep the original link.
| http://www.py2exe.org/ | ||
|
|
||
| Another tool is Anthony Tuininga's `cx_Freeze <http://cx-freeze.sourceforge.net/>`_. | ||
| Another tool is Anthony Tuininga's `cx_Freeze <https://anthony-tuininga.github.io/cx_Freeze/>`_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay.
Doc/faq/windows.rst
Outdated
|
|
||
| See http://cx-freeze.sourceforge.net/ for a distutils extension that allows you | ||
| to create console and GUI executables from Python code. | ||
| See https://anthony-tuininga.github.io/cx_Freeze/ for a distutils extension |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay.
Doc/faq/windows.rst
Outdated
| Simply rename the downloaded file to have the .TGZ extension, and WinZip will be | ||
| able to handle it. (If your copy of WinZip doesn't, get a newer one from | ||
| https://www.winzip.com.) | ||
| http://www.winzip.com/.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Insignificant change.
| BDFL | ||
| Benevolent Dictator For Life, a.k.a. `Guido van Rossum | ||
| <https://www.python.org/~guido/>`_, Python's creator. | ||
| <https://gvanrossum.github.io/>`_, Python's creator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay.
|
Hi @serhiy-storchaka I've done the changes. Also, for the other half, I've tried to do similar changes. Can you please have a look now? |
|
@serhiy-storchaka Can you please take a look here once? |
|
Hey @serhiy-storchaka , Just wanted to ping you to know if it would be possible for you to take a look here once. Thanks :) |
|
@CuriousLearner and @serhiy-storchaka, does this pull request aim to fix only link redirections, as the title says? Because in your first message you wrote: "This is a Work in Progress PR to address link fixes in documentation", and also bpo-25910 is focusing on Fixing links in documentation, so the goal is not clear to me. I am asking because if this PR wants to fix all documentation links, than we have to close #2765 . |
Doc/library/ssl.rst
Outdated
| .. data:: HAS_NPN | ||
|
|
||
| Whether the OpenSSL library has built-in support for *Next Protocol | ||
| Negotiation* as described in the `NPN draft specification |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link label is now outdated.
Doc/library/ssl.rst
Outdated
| ordered by preference. The selection of a protocol will happen during the | ||
| handshake, and will play out according to the `NPN draft specification | ||
| <https://tools.ietf.org/html/draft-agl-tls-nextprotoneg>`_. After a | ||
| <https://en.wikipedia.org/wiki/Application-Layer_Protocol_Negotiation >`_. After a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
|
I've just run |
Doc/whatsnew/2.3.rst
Outdated
|
|
||
| The idea of generators comes from other programming languages, especially Icon | ||
| (https://www.cs.arizona.edu/icon/), where the idea of generators is central. In | ||
| (https://www2.cs.arizona.edu/icon/), where the idea of generators is central. In |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep the original link. While it's currently a redirection to https://www2.cs.arizona.edu/icon/ we can still find https://www.cs.arizona.edu/icon/ on the site itself, but also on Wikipedia.
|
I found the following I suggest this based on the fact that:
For additional details, see: https://blog.readthedocs.com/securing-subdomains/
|
|
I also found the following links that should be updated to use https. |
|
Thanks for your inputs @jdufresne . I've fixed those things in @vstinner I'm not sure about this change: Apart from that, there are several redirection from Note: I've left the redirections for Github links; for which I'll file a separate PR. |
f3c2605 to
353d5bc
Compare
vstinner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not good yet. But this time, I added "ok" comment on each correct change. So the next review should be quicker for me.
FYI I copied/pasted each time manually to open them in Firefox, it took me "forever" to check all these links...
| recommended tools`_. | ||
|
|
||
| .. _currently recommended tools: https://packaging.python.org/en/latest/current/#packaging-tool-recommendations | ||
| .. _currently recommended tools: https://packaging.python.org/guides/tool-recommendations/#packaging-tool-recommendations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
|
||
| .. _Project structure: \ | ||
| https://packaging.python.org/en/latest/distributing/ | ||
| https://packaging.python.org/tutorials/distributing-packages/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| https://packaging.python.org/tutorials/distributing-packages/ | ||
| .. _Building and packaging the project: \ | ||
| https://packaging.python.org/en/latest/distributing/#packaging-your-project | ||
| https://packaging.python.org/tutorials/distributing-packages/#packaging-your-project |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| https://packaging.python.org/tutorials/distributing-packages/#packaging-your-project | ||
| .. _Uploading the project to the Python Packaging Index: \ | ||
| https://packaging.python.org/en/latest/distributing/#uploading-your-project-to-pypi | ||
| https://packaging.python.org/tutorials/distributing-packages/#uploading-your-project-to-pypi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
|
||
| `Python Packaging User Guide: Binary Extensions | ||
| <https://packaging.python.org/en/latest/extensions>`__ | ||
| <https://packaging.python.org/guides/packaging-binary-extensions/>`__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| PEP written by Aahz; implemented by Thomas Wouters. | ||
|
|
||
| https://pylib.readthedocs.org/ | ||
| https://pylib.readthedocs.io/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
|
||
| Hosting of the Python bug tracker is kindly provided by | ||
| `Upfront Systems <http://www.upfrontsystems.co.za/>`__ | ||
| `Upfront Systems <http://www.upfrontsoftware.co.za>`__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| to be allowed; the format of the string is described | ||
| `in the OpenSSL documentation | ||
| <https://www.openssl.org/docs/apps/ciphers.html#CIPHER-LIST-FORMAT>`__. | ||
| <https://www.openssl.org/docs/manmaster/man1/ciphers.html#CIPHER-LIST-FORMAT>`__. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| argument. The *ciphers* string lists the allowed encryption algorithms using | ||
| the format described in the `OpenSSL documentation | ||
| <https://www.openssl.org/docs/apps/ciphers.html#CIPHER-LIST-FORMAT>`__. | ||
| <https://www.openssl.org/docs/manmaster/man1/ciphers.html#CIPHER-LIST-FORMAT>`__. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| a class variable and should not be set on instances of that class. | ||
| (Contributed by Ivan Levkivskyi in `Github #280 | ||
| <https://github.com/python/typing/issues/280>`_.) | ||
| <https://github.com/python/typing/pull/280>`_.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
Hey @vstinner "FYI I copied/pasted each time manually to open them in Firefox, it took me "forever" to check all these links..." I really appreciate all the efforts you've put in to review this PR. Thank a lot. I've addressed the reviews, and now I think it would take a lot less time to review. Also, please see this query: #1933 (comment) |
In case of doubt, do nothing. I see that you kept https://www.python.org/psf/license/ : good. |
The author fixed issues reported by Serhiy
|
Thanks @CuriousLearner for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6. |
|
Sorry, @CuriousLearner and @vstinner, I could not cleanly backport this to |
|
I don't think that it's worth it to backport this change to Python 3.6 (I missed the "need 3.6 backport" label when I merged this PR). |
|
I merged your PR @CuriousLearner, thanks. |
|
Thanks a lot @vstinner :) |
This is a Work in Progress PR to address link fixes in documentation.
I intend to address url redirection issues in this PR.
Please let me know what changes are needed in this. Thanks!
https://bugs.python.org/issue25910