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

Sphinx 3.4.x compatibility #617

Merged
merged 3 commits into from
Jan 8, 2021
Merged

Conversation

utzig
Copy link
Contributor

@utzig utzig commented Jan 6, 2021

Allows for running on 3.4.x series, and updates test matrix to add Python 3.9.0 and all minor Sphinx releases in the 3.x series as well as the development tag.

This fixes #612 and supersedes #615

@vermeeren I would suggest releasing 4.26.0 if possible.

@utzig
Copy link
Contributor Author

utzig commented Jan 6, 2021

@vermeeren Also worth mentioning that I tested with Python 3.9.0 and Sphinx 3.4.2 on my projects with no issues.

@jakobandersen
Copy link
Collaborator

jakobandersen commented Jan 7, 2021

LGTM

@vermeeren vermeeren self-assigned this Jan 7, 2021
@vermeeren vermeeren added the packaging Requirements, setup.py, etc label Jan 7, 2021
@vermeeren vermeeren mentioned this pull request Jan 7, 2021
Copy link
Collaborator

@vermeeren vermeeren left a comment

Choose a reason for hiding this comment

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

Added a missing update to requirements/production.txt just now, everything else looks good. Thanks @utzig !

@utzig
Copy link
Contributor Author

utzig commented Jan 7, 2021

Added a missing update to requirements/production.txt just now, everything else looks good. Thanks @utzig !

Btw, Sphinx==3.4.3 came out today, I didn't have time to test it, but I can do some testing and update the test matrix now (in the next 1 hour or so I hope).

@vermeeren
Copy link
Collaborator

@utzig Strange that CI complains now, from https://github.com/michaeljones/breathe/pull/617/checks?check_run_id=1665421730

Run make type-check
mypy breathe tests
breathe/path_handler.py:18: error: Argument 1 to "join" has incompatible type "Optional[str]"; expected "Union[str, _PathLike[str]]"
breathe/directives.py:592: error: Argument 1 to "join" has incompatible type "Optional[str]"; expected "Union[str, _PathLike[str]]"
Found 2 errors in 2 files (checked 28 source files)
make: *** [type-check] Error 1
Makefile:48: recipe for target 'type-check' failed
Error: Process completed with exit code 2.

Sphinx 3.4.3 changes very little, see sphinx-doc/sphinx@v3.4.2...v3.4.3 . Thoughts on fixing this? Can always do # type: ignore I suppose.

And yes, I'll be glad to wait for your next version in the hour or so. Thanks!

Allow running with Sphinx releases in the 3.4.x series.

Signed-off-by: Fabio Utzig <fabio.utzig@nordicsemi.no>
This allows running tests on Python 3.9.0 and update the Sphinx releases
with all latest 3.x versions, as well as the proper minor development
tag.

Signed-off-by: Fabio Utzig <fabio.utzig@nordicsemi.no>
Fix calls to path.join where the appdir is detected by mypy has having
an invalid type; force mypy to ignore them for now.

Signed-off-by: Fabio Utzig <fabio.utzig@nordicsemi.no>
@utzig
Copy link
Contributor Author

utzig commented Jan 7, 2021

Sphinx 3.4.3 changes very little, see sphinx-doc/sphinx@v3.4.2...v3.4.3 . Thoughts on fixing this? Can always do # type: ignore I suppose.

Yeah, I fixed the mypy issues as you suggested; I guess it would be a good idea to understand why this happened, I can try to track it down tomorrow. But it works 100%, tested Sphinx==3.4.3 with the updated PR and it's all fine!

@vermeeren
Copy link
Collaborator

@utzig Thanks! I'll check it out and hopefully make a release tonight.

vermeeren added a commit that referenced this pull request Jan 7, 2021
@michaeljones michaeljones merged commit 8e6d775 into breathe-doc:master Jan 8, 2021
@jakobandersen
Copy link
Collaborator

Yeah, I fixed the mypy issues as you suggested; I guess it would be a good idea to understand why this happened

It looks like it is because of Sphinx.confdir which it thinks has type Optional[str]. However, after Sphinx.__init__() it can no longer be None so ignoring the type error seems reasonable to me.

@utzig utzig deleted the sphinx-3.4 branch January 8, 2021 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packaging Requirements, setup.py, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Request] Sphinx 3.4.x support
4 participants