Use READTHEDOCS_VERSION in docs to avoid 'unknown'#455
Use READTHEDOCS_VERSION in docs to avoid 'unknown'#455FlorianWilhelm merged 3 commits intomasterfrom
Conversation
|
btw, I enabled RTD to build for PRs, so we can also inspect documentation changes. We can see it now in the checks list as |
docs/conf.py
Outdated
| # | ||
| # The short X.Y version. | ||
| version = "" # Is set by calling `setup.py docs` | ||
| version = os.getenv("READTHEDOCS_VERSION", "") |
There was a problem hiding this comment.
Who is setting this environment variable? I think we should make sure that git tag versions = pypi distributions = the rtd versions.
There was a problem hiding this comment.
I would rather completely remove this part as we set version later using __vesion__. This only confuses people.
There was a problem hiding this comment.
So this env variable is set in all environments by Read The Docs.
The reason why I added it here is to have a fallback mechanism when publishing the docs, in the case there is a problem with the import statement.
There was a problem hiding this comment.
Ok, should we add a comment then that it's later potentially overwritten? Or remove it at this point and put it in the try-except block directly? It might be confusing to readers who read version = and think, okay so it's an environment variable but later it's overwritten.
There was a problem hiding this comment.
I agree with the confusion, in the latest commit to this PR I collapsed both sections of the code manipulating version/release into a single one. Hopefully it avoids confusion.
Python 3.9 is not supported yet... When it is, it is always better to bump, since type hint related features of sphinx and its extensions work better the higher the Python version.
| pass | ||
| else: | ||
| release = version | ||
| # html_title = None |
There was a problem hiding this comment.
The preceding original comment was not related to version/release.
Apparently this html_title = None was deleted by mistake at some point in the history of the file.
| if not version or version.lower() == "unknown": | ||
| version = os.getenv("READTHEDOCS_VERSION", "unknown") # automatically set by RTD | ||
|
|
||
| release = version |
There was a problem hiding this comment.
Now everything is centralised in a single section of the code.
|
Thank you very much for the review @FlorianWilhelm. I tried to incorporate your comments in the latest changes, in special:
|
abravalheri
added
the
documentation
|
Thanks @abravalheri, looks really good! |
2 participants
Fixes #454.