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.

Sign up for GitHub

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

Jump to bottom

docs: Use intersphinx to map old versions and cleanup version history #16155

Merged
merged 8 commits into from Apr 27, 2021
Merged

docs: Use intersphinx to map old versions and cleanup version history #16155

merged 8 commits into from Apr 27, 2021

Conversation

phlax
Copy link
Member

@phlax phlax commented Apr 24, 2021 •
edited

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message: docs: Use intersphinx to map old versions and cleanup version history
Additional Description:

This PR

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

@phlax phlax marked this pull request as draft April 24, 2021 08:17
@phlax phlax changed the title [WIP] docs: Use intershpinx to map old versions docs: Use intershpinx to map old versions Apr 25, 2021
@phlax phlax changed the title docs: Use intershpinx to map old versions docs: Use intersphinx to map old versions Apr 25, 2021
@phlax phlax marked this pull request as ready for review April 25, 2021 09:23
@phlax phlax requested a review from htuch April 25, 2021 09:23
@phlax phlax changed the title docs: Use intersphinx to map old versions [WIP] docs: Use intersphinx to map old versions Apr 25, 2021
@phlax phlax marked this pull request as draft April 25, 2021 12:36
@phlax
Copy link
Member Author

phlax commented Apr 25, 2021

need to investigate an issue with intersphinx...

@phlax
Copy link
Member Author

phlax commented Apr 25, 2021 •
edited

it seems that intersphinx has a very unhelpful "feature" (or > 6-year-old bug depending on pov sphinx-doc/sphinx#2068) by which it kinda guesses which set of docs a link belongs to.

this means that by enabling intersphinx any broken links in current docs will happily pass if it can find the link in any of the linked documentation

@nijel from weblate has submitted a patch to fix this (here - sphinx-doc/sphinx#8981) but afaict the sphinx devs either dont understand this bug (and how it makes intersphinx basically unusable - at least for versioning) or dont care too much (they are +-0 to fixing ?) and it wont be fixed until at least sphinx==4.1

for these reasons i have included a patched version of intersphinx with @nijel 's fix

@nijel
Copy link

nijel commented Apr 25, 2021

Please comment on the pull request so that Sphinx developers can see there are more people wanting this ;-)

@phlax phlax force-pushed the docs-version-histories branch 5 times, most recently from fc756e0 to 4025cf6 Compare April 25, 2021 15:14
@phlax phlax changed the title [WIP] docs: Use intersphinx to map old versions [WIP] docs: Use intersphinx to map old versions and cleanup version history Apr 25, 2021
@phlax phlax force-pushed the docs-version-histories branch 2 times, most recently from 3349bbb to cffb110 Compare April 25, 2021 19:08
@phlax phlax changed the title [WIP] docs: Use intersphinx to map old versions and cleanup version history docs: Use intersphinx to map old versions and cleanup version history Apr 25, 2021
@phlax phlax marked this pull request as ready for review April 25, 2021 19:09
@phlax
Copy link
Member Author

phlax commented Apr 25, 2021

docs are rendered here https://storage.googleapis.com/envoy-pr/16155/docs/index.html

@phlax phlax force-pushed the docs-version-histories branch 5 times, most recently from 87dd781 to d5ec851 Compare April 25, 2021 19:59
@htuch
Copy link
Member

htuch commented Apr 25, 2021

@phlax what do you think the likelihood of getting this fixed upstream and landed soon is? I'm reluctant to take on this fork, but I also understand the concern.

@phlax
Copy link
Member Author

phlax commented Apr 26, 2021 •
edited

... I also understand the concern.

Yep - i think we definitely want to use this and it definitely has this bug/issue

That means we have to choose the least bad option. The ones i considered

  • live with the bug and hope we dont accidentally add/remove link targets
  • build the docs twice - first removing the extension and all version info, and then readding it
  • create a separate intersphinx package somewhere
  • dynamically patch the module during docs build - ie curl ... > _ext/intersphinx.py && patch ...
  • copy the module into the repo and just add the fix

of those options the last seems the most straightforward - its ultimately a single module/file that had to be copied in

i can add an Envoy ticket to remind us to remove it and use upstream once the issue is fixed. We may have to update it anyway if we upgrade to sphinx 4

what do you think the likelihood of getting this fixed upstream and landed soon is?

sphinx 4 is currently in beta - not sure of the timescales to release. The PR for this was bumped 4 -> 4.1 - not sure why - it could just be that it needs rebasing (there were some very minor nits from my reading - which may/not be valid - tbh i dont understand why it wasnt just landed at the time)

@phlax
Copy link
Member Author

phlax commented Apr 26, 2021 •
edited

relatedly, ive begun testing the sphinx4 upgrade

atm it blocks on sphinx-tabs - there is a PR with compat here executablebooks/sphinx-tabs#110 - so afaict once sphinx4 is out of beta that should unblock

htuch
htuch reviewed Apr 26, 2021
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM modulo comment.

@phlax
docs: Use intersphinx to map old versions
6f8f3b0 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
intersphinx_custom
d60b722 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
inline-literals
6306f63 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
add workaround comment
ce24661 
Signed-off-by: Ryan Northey <ryan@synca.io>
htuch
htuch previously approved these changes Apr 27, 2021
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@htuch
Copy link
Member

htuch commented Apr 27, 2021

@phlax needs format fix.

@phlax
Copy link
Member Author

phlax commented Apr 27, 2021

seems to have format issues - i will fix tomorrow

/wait

@phlax
Merge branch 'main' into docs-version-histories
da6ac26 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
format
1cceb05 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
merge
cc78aff 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
fix-v1.8
6436dd9 
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax mentioned this pull request Apr 27, 2021
Merged
@htuch htuch merged commit 4a0dfd5 into envoyproxy:main Apr 27, 2021
gokulnair pushed a commit to gokulnair/envoy that referenced this pull request May 6, 2021
@phlax
docs: Use intersphinx to map old versions and cleanup version history (…
972758d 
…envoyproxy#16155)

This PR
- adds a patched version of intersphinx (~from sphinx-doc/sphinx#8981)
- uses versioned refs for version history
- cleans up inline literals in version history

Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Gokul Nair <gnair@twitter.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@htuch htuch htuch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@phlax @nijel @htuch