readthedocs / sphinx-hoverxref

Sphinx extension to show tooltips with content embedded when hover a reference.
https://sphinx-hoverxref.readthedocs.io/
MIT License
96 stars 40 forks source link

Dependencies: add minimum docutils version #281

Closed EFord36 closed 6 months ago

EFord36 commented 6 months ago

Closes #280

Versions lower than 0.20 cause rendering problems in the usage page with the bibliography, see #280.


📚 Documentation preview 📚: https://sphinx-hoverxref--281.org.readthedocs.build/en/281/

EFord36 commented 6 months ago

the preview doesn't show this as fixed - but looking at the html from the preview, it's still using docutils 0.18.0:

<meta name="generator" content="Docutils 0.18.1: http://docutils.sourceforge.net/">

so the workflow must be caching the dependencies somehow.

humitos commented 6 months ago

The requirements are installed from docs/requirements.txt. See https://beta.readthedocs.org/projects/sphinx-hoverxref/builds/23418992/#219443307--14

EFord36 commented 6 months ago

oops! Thanks for explaining, I'll run the pip-compile command and update the PR

EFord36 commented 6 months ago

Updated, and now the preview is fixed as expected 😄 Thanks for the help on the CI side, sorry for not noticing that requirements.txt file myself!

humitos commented 6 months ago

Thanks for the PR. No need to sorry 😉

Tests are broken because Sphinx<5 is broken due to their own dependencies (see https://github.com/sphinx-doc/sphinx/issues/11890). Not sure what to do there -- we can probably pin all their dependencies to older versions, maybe?

EFord36 commented 6 months ago

Thanks for the PR. No need to sorry 😉

Tests are broken because Sphinx<5 is broken due to their own dependencies (see sphinx-doc/sphinx#11890). Not sure what to do there -- we can probably pin all their dependencies to older versions, maybe?

Happy to add the pins for the relevant dependency - I just saw #279 though and your comment about doing a new release for Sphinx>=5.0 - I think maybe that's the better solution? Most end users will be using sphinx >5 anyway, so it would save force users for whom everything is working fine from using an older version of sphinx's dependencies than necessary for them.

what would you think of merging this pr 'as-is', or potentially modifying the tox setup to not test Sphinx versions below 5.0, and then following up with a release requirement Sphinx 5.0 or newer?

humitos commented 6 months ago

what would you think of merging this pr 'as-is', or potentially modifying the tox setup to not test Sphinx versions below 5.0, and then following up with a release requirement Sphinx 5.0 or newer?

I think we should do all of that, yes 😄

Would you like to work on these? I'm happy to continue reviewing and helping you here.

EFord36 commented 6 months ago

what would you think of merging this pr 'as-is', or potentially modifying the tox setup to not test Sphinx versions below 5.0, and then following up with a release requirement Sphinx 5.0 or newer?

I think we should do all of that, yes 😄

  • Merge this PR
  • Update tox to remove old tests
  • Release a new version that only supports Sphinx>=5

Would you like to work on these? I'm happy to continue reviewing and helping you here.

Sure - I'll start work on a PR to update tox and update the pyproject.toml and changelog etc. for the new release.

For the actual release, shall I leave the running of the final flit publish to you?

For this PR, are you happy to merge now, or do you want to wait for the tox test update?

humitos commented 6 months ago

For the actual release, shall I leave the running of the final flit publish to you?

Yes.

For this PR, are you happy to merge now, or do you want to wait for the tox test update?

Let's wait for the tox updates so we are sure everything is on it's place 👍🏼

EFord36 commented 6 months ago

@humitos I've updated this now, and all looks ok to me (assuming the tests pass), let me know if there's anything else you want me to do on it!