Skip to content
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

PEP 0: Make PEP titles in table clickable links #2429

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

CAM-Gerlach
Copy link
Member

As requested by @gvanrossum in #2364 ,

FWIW the one usability improvement you could make in PEP 0 with the biggest effect would be to make PEP titles clickable. Having to aim at the little number in front of e.g. PEP 1 is awkward for someone with as clumsy a mouse as myself.

This PR does exactly that.

@CAM-Gerlach CAM-Gerlach added the infra Core infrastructure for building and rendering PEPs label Mar 14, 2022
@CAM-Gerlach CAM-Gerlach self-assigned this Mar 14, 2022
@CAM-Gerlach CAM-Gerlach requested a review from AA-Turner as a code owner March 14, 2022 19:11
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Looks good! Please could you also update the docstring on line 24?

@CAM-Gerlach CAM-Gerlach force-pushed the pep-0-make-titles-clickable branch from 662aac6 to 25447c2 Compare March 14, 2022 19:32
@CAM-Gerlach
Copy link
Member Author

Please could you also update the docstring on line 24?

Sure, done.

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Mar 14, 2022

Random side question @hugovk , but is there a reason the RTD deploy previews take so long (and provide relatively little feedback while they're building)? They take a full 3.5 minutes to generate and upload, which is over twice the time of the actual GH pages builds at 1.5 minutes (despite the latter using more rigorous, and thus slower, python and sphinx options), Using other services like Netlify, I've never had the deploy previews take more than a few seconds longer than the main build, and sometimes they were even faster (and I'm also used to much more real-time output than I'm seeing here).

@hugovk
Copy link
Member

hugovk commented Mar 15, 2022

Looking at some logs:


The slowest bit is the Sphinx build, and isn't using Sphinx's -j flag to build in parallel:

[rtd-command-info] start-time: 2022-03-15T01:08:14.980982Z, end-time: 2022-03-15T01:09:37.696641Z, duration: 82, exit-code: 0
/home/docs/checkouts/readthedocs.org/user_builds/pep-previews/envs/latest/bin/python -m sphinx -T -E -b dirhtml -d _build/doctrees -D language=en . _build/html
Running Sphinx v4.4.0

I see there's a SPHINX_PARALLEL feature flag we could ask RTD to enable:


The next slowest is installing dependencies (RTD docs):

[rtd-command-info] start-time: 2022-03-15T01:07:41.295093Z, end-time: 2022-03-15T01:08:10.005945Z, duration: 28, exit-code: 0
/home/docs/checkouts/readthedocs.org/user_builds/pep-previews/envs/latest/bin/python -m pip install --upgrade --no-cache-dir mock==1.0.1 pillow==5.4.1 alabaster>=0.7,<0.8,!=0.7.5 commonmark==0.8.1 recommonmark==0.5.0 sphinx sphinx-rtd-theme readthedocs-sphinx-ext<2.2

One thing that sticks out there is the pillow==5.4.1 pin and we're running on Python 3.9. The first version to explicitly support Python 3.9 with prebuilt wheels is Pillow 8, so it's building 5.4.1 from source.

We can require dependencies in the config file but I think it installs those after the default ones. Worth a try though.

The newest Python supported by 5.4.1 is 3.7, and we need 3.9, so probably makes more sense to ask RTD to upgrade instead, or allow skipping it if not needed.

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Mar 15, 2022

Thanks; I'm just used to having full control over the build environment, and it matching between my CIs, rather than it being managed for me. I assumed they'd install the project's deps from the same requirements file, but it seems like they are installing a whole heavy stack of others instead—and since they are managing it and pinning everything, and to old versions at that, I wonder why they aren't just cached in the environment?

I was previously in favor of standardizing on RTD, and hopefully at least some of these issues can get speedily resolved, but if not maybe we should consider switching to something like Netlify for the previews (and retain the existing GHP setup for deploy), which would offer the same preview functionality but allow us to run the exact same code we do in our GitHub Actions, for full control, speed and consistency between builders. But we should presumably defer any such decision until Adam gets back and we discuss it with the team and community.

@CAM-Gerlach
Copy link
Member Author

As this is a straightforward tweak and its been 10 days, I'll go ahead and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA signed infra Core infrastructure for building and rendering PEPs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants