GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix refs section removal breaking external links on Sphinx build too by CAM-Gerlach · Pull Request #2229 · python/peps

Conversation

@CAM-Gerlach

@CAM-Gerlach CAM-Gerlach commented Jan 12, 2022

Copy link
Copy Markdown
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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@CAM-Gerlach
Fix refs section removal breaking external links on Sphinx build too
65cdd0a
@CAM-Gerlach
CAM-Gerlach requested a review from a team as a code owner January 12, 2022 02:31
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jan 12, 2022
@CAM-Gerlach

CAM-Gerlach commented Jan 12, 2022

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra
JelleZijlstra approved these changes Jan 12, 2022
View reviewed changes
@JelleZijlstra
JelleZijlstra merged commit 57c9a36 into python:main Jan 12, 2022
@JelleZijlstra

JelleZijlstra commented Jan 12, 2022

Copy link
Copy Markdown
Member

It worked :)

CAM-Gerlach reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra

JelleZijlstra commented Jan 12, 2022

Copy link
Copy Markdown
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 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@CAM-Gerlach

CAM-Gerlach commented Jan 12, 2022

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pradyunsg

pradyunsg commented Jan 12, 2022

Copy link
Copy Markdown
Member

And, the re-run seems to have succeeded!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AA-Turner

AA-Turner commented Jan 12, 2022

Copy link
Copy Markdown
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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

CAM-Gerlach commented Jan 12, 2022

Copy link
Copy Markdown
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).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@CAM-Gerlach CAM-Gerlach mentioned this pull request Feb 4, 2022
PEP 12: Recommend and explain native RST hyperlinks (versus footnotes) #2302
Merged
@erlend-aasland erlend-aasland mentioned this pull request May 13, 2022
pep8/greppable exception messages erlend-aasland/peps#1
Closed
@erlend-aasland erlend-aasland mentioned this pull request Jun 27, 2022
pep 687/mark as accepted erlend-aasland/peps#2
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

CLA signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@CAM-Gerlach @JelleZijlstra @pradyunsg @AA-Turner @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.