GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||
GeezerVison is showing github.com — bpo-37860: re-add netlify.toml to set up deploy previews for docs by epicfaace · Pull Request #92852 · python/cpython Conversation
Contributor
Fixes #82041 by setting up Netlify to set up deploy previews for docs. This PR re-does #15288. Reverts #30272 (where we had originally removed netlify.toml due to build limits). As per #82041 (comment), the build limits are now large enough that they should be fine to use for cpython.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
This reverts commit fbaf2e6.
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
This was referenced
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
|
There was an error while loading. Please reload this page.
This is already configured by default. The link above says "Depending on your Branches settings, any time there is a change in a linked repository, Netlify tries to determine whether there are changes in the site’s base directory by comparing the last known version of the files in that directory." The |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Great, then I think we're all set! @ewdurbin As a follow on to #15288 (comment) in 2019, we should now have 25k minutes per month (#82041 (comment)), and are ready to try this out again. Please could you hook up the Netlify account? I would suggest we only run it on If we have max 3 team members (#15288 (comment)), @Mariatta, you asked to be added in 2019 (#15288 (comment)), would you still like to? And to further increase the bus factor, who would like to be the 3rd? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I'm happy to be the third! If it's fine though I'm not a committer |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Also, looks like we're no longer limited to 3 team members max: see https://www.netlify.com/legal/open-source-policy/:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Also, one more thing: I think to comply with the requirements mentioned in https://www.netlify.com/legal/open-source-policy/, we should add a Netlify badge / "This site is powered by Netlify" message on all deploy previews. I don't think we need to / should add it to the main docs, but we can add it on deploy previews only -- for example, we could set an environment variable that then renders the Netlify badge only on deploy previews. I'm happy to implement if we're good with that approach. What do you think @hugovk ? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The env var approach sounds good to add it only to deploy previews. Perhaps the text link option, in the footer? And just on the homepage or all pages? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for your work @epicface. I think we need to display more prominently that the docs rendered is a "preview". So where-ever we put the netlify badge, we should add the "preview" info as well. Is that something we can do? As comparison, on the devguide (built with readthedocs), the preview is displayed at the top of the page: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Adding a warning admonition to the top of the page seems a reasonable idea regardless, although the ephemeral nature of the previews (& non-canonical URL) should help to disambiguate d.p.o from the previews. @hugovk from the linked Netifly page, it seems only the "main page" is required - so A |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hi again @epicfaace! Do you think you can make the above changes? Thanks! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I think the popup warning that collapses into the bottom will be more noticeable. Here is an example from tc39/ecma262: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yes, doing so! @arhadthedev I like that idea. relevant code here: https://github.com/tc39/ecma262/blob/267fdbdc716677bc9934906a498cf9bad4bfb70d/scripts/snapshot_warning.html https://github.com/tc39/ecma262/blob/267fdbdc716677bc9934906a498cf9bad4bfb70d/scripts/insert_snapshot_warning.js and https://github.com/tc39/ecma262/blob/267fdbdc716677bc9934906a498cf9bad4bfb70d/scripts/snapshot_warning.css |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
All right, this PR should be ready now, @Mariatta and @hugovk . I just added a simple banner for now that displays on top of all pages: Sample deploy preview: https://62af84a185fcf70008d07339--steady-heliotrope-731cac.netlify.app/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks, a couple of comments!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <[email protected]>
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Note that this banner uses the same styling as the "This document is for an old version of Python that is no longer supported" banner. The top of this banner will also be hidden once we start showing this banner on versions of the docs that have a responsive mobile layout. This means that when 3.8 reaches EOL (as 3.8 is the first version of the docs with a responsive mobile layout), we'll have a similar problem in which the top of the banner "This document is for an old version of Python that is no longer supported." will be hidden. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
to clarify -- 3.6 doesn't have this problem as it's not using a mobile responsive layout: but 3.8 most certainly will, once we add the red banner. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ah okay. So for the "no longer supported" it won't happen until 3.8 is EOL on 2024-10-14, but for the previews it will happen right away :) We could report this to (I guess) https://github.com/python/python-docs-theme and come up with a fix later? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@hugovk yes. It seems more involved to fix that issue so I've went ahead and reported it in python/python-docs-theme#95. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@ewdurbin Hi! Please could you hook up the Netlify account again? See #92852 (comment) And please add @Mariatta to the team. Thank you! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@ewdurbin Will you be able to do this, or is something blocking you? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Looks like we're all set now! Here's a recent docs PR build: And the corresponding preview: Thank you Ee for setting up Netlify and Ashwin for this and the earlier PRs! 👏 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The banner is missing, though. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
You're right, with an up-to-date branch it works! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This is amazing! Thank you everyone for pushing through and thanks @epicfaace for this PR. This is a great help to all contributors here. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Amazing, thank you all for reviewing and getting this in! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Add deploy preview for docs
There was an error while loading. Please reload this page.