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

Fix refs section removal breaking external links on Sphinx build too #2229

Merged
merged 1 commit into from
Jan 12, 2022

Conversation

CAM-Gerlach
Copy link
Member

Followup to #2227 to fix #2155 for the new Sphinx build; I'd previously attempted to reproduce the issue locally on the same PEP and it apparently didn't occur, but I didn't realize I was looking at a stale built file, and it turns out that in fact the same fix is needed. Sorry for the double PR!

@CAM-Gerlach
Copy link
Member Author

@JelleZijlstra Seems like the same thing is happening here. Funny enough, the lint job succeeded and the build job failed here, versus the opposite happened on #2228 despite being re-run at the same time, so seems pretty random. If you restart the failing workflows, seems like there's a chance it succeeds.

@JelleZijlstra JelleZijlstra merged commit 57c9a36 into python:main Jan 12, 2022
@JelleZijlstra
Copy link
Member

It worked :)

@JelleZijlstra
Copy link
Member

But the deploy job failed: https://github.com/python/peps/actions/runs/1685665720. Let's just give GH some time to fix things and maybe rerun it later.

@CAM-Gerlach
Copy link
Member Author

Looks like it has been resolved. Could you re-run?

@pradyunsg
Copy link
Member

And, the re-run seems to have succeeded!

@AA-Turner
Copy link
Member

Thanks for catching this Cam/all -- docutils can be annoyingly fickle sometimes.

As a post-mortem, what we should have done initially is made all target children of the references section children of the document.

Deferring the transform is probably the smallest change to fix it though, and we don't rely on any behaviour transforming the footer links, so all good. Thanks again!

A

@CAM-Gerlach CAM-Gerlach deleted the fix-refs-sphinx-build branch January 12, 2022 18:54
@CAM-Gerlach
Copy link
Member Author

@AA-Turner Yup, great point—decreasing the priority to an arbitrarily low value felt kinda hacky, and I thought about just moving the link targets somewhere else, but given it didn't appear to be necessary, would be a much bigger change and I didn't want to mess that up, plus I wanted to get the important fix out as quickly as practical, so I ended up going with it (I also initially thought it only affected the legacy build system, since I didn't realize I was looking at a stale rendered file, so I wasn't as concerned with implementing a hacky solution).

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.

5 participants