Skip to content

[2.7] Doc: Add an optional obsolete header. (GH-13638) #19229

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

Merged
merged 9 commits into from
Apr 18, 2020

Conversation

leonardr
Copy link

(ported from commits 46ed90d and 00f46db)

Co-authored-by: Julien Palard julien@palard.fr

My apologies for the multiple PRs; it took me a couple tries to get this right.

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@leonardr

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@brainwane
Copy link
Contributor

@benjaminp Please review. Thank you.

<div id="outdated-warning" style="padding: .5em; text-align: center; background-color: #FFBABA; color: #6A0E0E;">
{% trans %}This document is for an old version of Python that is {% endtrans %}<a href="https://devguide.python.org/devcycle/#end-of-life-branches">{% trans %}no longer supported{% endtrans %}</a>.
{% trans %}You should upgrade, and read the {% endtrans %}
<a href="/3/{{ pagename }}{{ file_suffix }}">{% trans %} Python documentation for the current stable release{% endtrans %}</a>.
Copy link
Member

Choose a reason for hiding this comment

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

Could you try adding {{ language or 'en' }} like below on line 8/18?

Copy link
Author

Choose a reason for hiding this comment

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

I tried this out and it almost works, but https://docs.python.org/en/3/ isn't a working URL the way https://docs.python.org/ja/3/ and https://docs.python.org/pt-br/3/ are.

I used the ternary operator to get something which is unelegant but works. I tested it three ways:

  • with no language set: make html 'SPHINXOPTS=-A outdated=1'
  • with the language set to 'en': make html 'SPHINXOPTS=-A outdated=1 -A language=en'
  • with the language set to a value other than 'en': make html 'SPHINXOPTS=-A outdated=1 -A language=pt-br'

Let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds great to me, but I’m not the expert on Python docs translations, I’ll tag Julien.

@merwok merwok requested a review from JulienPalard April 16, 2020 21:17
<div id="outdated-warning" style="padding: .5em; text-align: center; background-color: #FFBABA; color: #6A0E0E;">
{% trans %}This document is for an old version of Python that is {% endtrans %}<a href="https://devguide.python.org/devcycle/#end-of-life-branches">{% trans %}no longer supported{% endtrans %}</a>.
{% trans %}You should upgrade, and read the {% endtrans %}
<a href="/{{ language + '/' if language and language != 'en' else '' }}3/{{ pagename }}{{ file_suffix }}">{% trans %} Python documentation for the current stable release{% endtrans %}</a>.
Copy link
Member

Choose a reason for hiding this comment

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

This href needs to be an absolute URL to the website and not just relative because the docs are also produced as a downloadable set of files for a particular release and thus need to work when browsing via files. Try building the docs and then opening the built file directly in a browser (for example. on macOS, if you were in the Doc directory, you could just type 'open build/html/index.html')

Doc/README.txt Outdated
Deprecation header
==================

You can define the ``outdated`` variable in ``html_context`` to show a
Copy link
Member

Choose a reason for hiding this comment

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

Since we're merging this change into 2.7 at the time it is being end-of-lifed, the "outdated" variable should be always set when building the 2.7 docs and not depend on being passed in from the "make" command. (We also build the docs in various ways so it would be problematic to modify all the make calls.) I'm not sure where the best place to put the definition is; just so it ends up in the branch.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@leonardr
Copy link
Author

I have made the requested changes; please review again.

  • The link template in Doc/tools/templates/layout.html is now absolute.
  • I've reworded Doc/README.txt to convey that the banner is to be expected and it's not necessary to do anything special to get it to show up.
  • outdated is now set in Doc/conf.py This means it's now possible to build the docs without specifying -A outdated=1. I tested it with these two commands:
make html
make html 'SPHINXOPTS=-A language=pt-br'

(The setting of outdated is also handled by my docsbuild-scripts PR, which can be extended to other releases as support for them ends.)

@bedevere-bot
Copy link

Thanks for making the requested changes!

@ned-deily: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from ned-deily April 17, 2020 23:23
Copy link
Member

@ned-deily ned-deily left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! Those look good to me. Note, I'm not that familiar with how all the internationalization works so it would still be good to have @JulienPalard review this.

@benjaminp benjaminp merged commit 0fc82e9 into python:2.7 Apr 18, 2020
@leonardr leonardr deleted the backport-46ed90d-2.7 branch April 18, 2020 23:01
@ncoghlan ncoghlan requested a review from benjaminp April 19, 2020 07:00
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.

7 participants