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

docs: update to sphinx 5.1 #565

Merged
merged 23 commits into from Jul 26, 2022
Merged

docs: update to sphinx 5.1 #565

merged 23 commits into from Jul 26, 2022

Conversation

onerandomusername
Copy link
Member

Summary

depends on #564

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running task lint
    • I have type-checked the code by running task pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@onerandomusername onerandomusername added t: documentation Improvements or additions to documentation/examples t: dependencies Addition/update/removal of dependencies labels Jun 12, 2022
@onerandomusername onerandomusername added this to the disnake v2.6 milestone Jun 12, 2022
@onerandomusername onerandomusername added the s: needs review Issue/PR is awaiting reviews label Jun 12, 2022
@shiftinv shiftinv changed the title Docs/sphinx 5.0 docs: update to sphinx 5.0 Jun 12, 2022
@onerandomusername onerandomusername changed the base branch from master to ci/require-docs-no-warns June 12, 2022 20:21
Base automatically changed from ci/require-docs-no-warns to master June 12, 2022 21:39
Copy link
Member

@shiftinv shiftinv left a comment

Choose a reason for hiding this comment

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

This currently has three (mostly cosmetic) issues on the search results page caused by upstream bugs in sphinx 5.0, one of which was fixed in 8841d6a. The other two might require reintroducing a patched searchtools.js, depends on how soon a new version would be released; I'll open an issue and see what we can do in the meantime.

edit: sphinx-doc/sphinx#10548

`result` has 6 elements, not 5. Doesn't matter at runtime, but might as well do it right
@shiftinv shiftinv added the s: blocked Issue/PR is blocked by other issues label Jun 13, 2022
@onerandomusername onerandomusername removed the s: blocked Issue/PR is blocked by other issues label Jun 22, 2022
This reverts commit a097f33.

no longer necessary with fixed searchtools.js
Since sphinx no longer uses jQuery's `ready`, we'd be racing two `DOMContentLoaded`
events, with sphinx's listener always firing first and invoking the search scorer
without us having set a `pattern`.
Instead, we now initialize `pattern` when we need it instead of assuming that
it was set already.
@shiftinv shiftinv added p: low Low priority s: blocked Issue/PR is blocked by other issues and removed s: blocked Issue/PR is blocked by other issues labels Jun 30, 2022
@onerandomusername
Copy link
Member Author

now waiting on sphinx-doc/sphinx#10657 before updating.

@onerandomusername onerandomusername removed the s: blocked Issue/PR is blocked by other issues label Jul 25, 2022
@onerandomusername onerandomusername dismissed shiftinv’s stale review July 25, 2022 02:25

sphinx has been updated

@onerandomusername
Copy link
Member Author

We should read through the the changelog and see if there are any other options we can benefit from aside from removing the patched searchtools.

https://www.sphinx-doc.org/en/master/changes.html

@onerandomusername onerandomusername changed the title docs: update to sphinx 5.0 docs: update to sphinx 5.1 Jul 25, 2022
@shiftinv
Copy link
Member

We should read through the the changelog and see if there are any other options we can benefit from aside from removing the patched searchtools.

As far as I can tell, there's only sphinx-doc/sphinx@95b8183 which may help avoid this (technically a workaround):

"enable_search_shortcuts": True,

Additionally, there's readthedocs/sphinx-hoverxref@0250263 which was added in hoverxref v1.1.2, we'll have to update our custom css to use the new selectors. May also want to consider using == instead of ~=, since it doesn't really seem to follow semver (I'd consider this a breaking change but it was added in a patch version /shrug).

@onerandomusername
Copy link
Member Author

I think i fixed both of those issues in 15535dc and 0f14d8d

@onerandomusername onerandomusername added p: high High priority and removed p: low Low priority labels Jul 26, 2022
changelog/565.doc.rst Outdated Show resolved Hide resolved
@shiftinv shiftinv merged commit d0a2855 into master Jul 26, 2022
@shiftinv shiftinv deleted the docs/sphinx-5.0 branch July 26, 2022 22:27
@shiftinv shiftinv removed the s: needs review Issue/PR is awaiting reviews label Jul 27, 2022
@onerandomusername onerandomusername added s: in progress Issue/PR is being worked on s: waiting for api/docs Issue/PR is waiting for API support/documentation and removed s: in progress Issue/PR is being worked on s: waiting for api/docs Issue/PR is waiting for API support/documentation labels Jul 28, 2022
onerandomusername added a commit that referenced this pull request Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: high High priority t: dependencies Addition/update/removal of dependencies t: documentation Improvements or additions to documentation/examples
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants