Restrict mkdocstrings and mkdocstrings-python version range#155
Restrict mkdocstrings and mkdocstrings-python version range#155Baharis merged 3 commits intoinstamatic-dev:mainfrom
Conversation
|
I confirm that with |
stefsmeets
left a comment
There was a problem hiding this comment.
Good catch, mkdocs isn't being developed anymore and the developers of mkdocs-material and mkdocstrings started a fork of mkdocs: https://github.com/zensical/zensical
Long term (maybe not that long), once zensical matures it should be relatively straightforward to migrate. Although there is nothing wrong with mkdocs =)
Seems like this particular issues is caused by these lines:
Lines 72 to 76 in 941750b
Where 'import' should be changed to 'inventories', could you see if that fixes the issue?
I just looked and you have maintainer rights on readthedocs. Your PR is from your personal account, that's why it probably does not show up. I usually just enable a version (linked to the branch) when I'm debugging a rtd build in a PR. The new rtd dashboard is a bit confusing to me, but you can just click a branch and tick active+hidden to enable it. It must be a branch on the instamatic repo though. I avoid building the docs in the PR as a check, because it's a waste of resources in 9/10 cases and if it catches anything it's usually not related to the PR. |
|
@stefsmeets I do have maintainer permissions on readthedocs, that's why I could start compiling docs for PR in the first place, but from what I read, adding a feedback into the PR reviews on GitHub requires some administrative work on GitHub. It is reasonable to check docs status using separate docs version, but as you mentioned the branch needs to be on instamatic-dev/, which effectively prevents anyone other than you from using this feature. Meanwhile anyone can create a PR, and what is the point of making sure everyone documents their features if we don't know whether the documentation will be updated. As you mentioned, in 9/10 cases PRs do not introduce any significant changes to documentation, but like in the recent case sometimes the documentation stops compiling not due to changes to documentation, but due to issues with back end. We spent 2 months not knowing the docs did not update, and a few 30-second compilations on each PR would show that to us clearly. I would argue a proper integration and adding a big green light @ PR signaling docs are fine is anything but a waste of resources. |
|
After replacing |
Can you not push branches? |
|
I can push branches to instamatic-dev/ using PRs, not sure if I can do it directly though - likely I can, but it feels like a bad practice to push a branch to another repo only to take a look if the documentation builds - aspecially if the alternative is an auto-build on PR. One more argument concerning resources: the community version of Read the docs is free and they literally give you a built-in option to build on PR, I don't feel like we need to be super conscious about resource use here: https://docs.readthedocs.com/platform/latest/pull-requests.html. |
|
Ah, I see. I always branch directly from the main repo. If you need it to work effectively then please do so. I tend to reduce my CI usage where I can, mostly in attempt to reduce my environmental impact. One example is the test workflow that only runs when a PR is marked as 'ready for review' and aggressive caching where possible (I also don't like waiting 😅 ). No need to run the entire test suite for every commit in a PR. It kills me when I fix a small typo and the entire test suite runs for 5 python versions. I use it mostly to sign off on the final check before merging. |
|
As mentioned, I myself apparently don't have enough permissions to add readthedocs "check" in GitHub. For the time being all I can do is enable or disable auto-docs on PR in readthedocs only. Having a "check" included with PR would require someone with GitHub admin privileges to add this. Since I do not want to force anything upon yourself, and made this PR primarily to fix the docs, I will merge this request and leave any potential changes to the checks for the next one. |
After merging #154 yesterday I noticed that the docs are not being compiled properly. It seems that recently both mkdocstrings and mkdocstrings-python increased their major version, and the new combination fails with the following exception. I configured read the docs to trigger documentation build on every PR and restricted the version range of both packages to address the issue: