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

Revisit auto documentation generation with sphinx on RTD #871

Merged
merged 7 commits into from
Jun 5, 2023

Conversation

cderici
Copy link
Contributor

@cderici cderici commented Jun 2, 2023

Description

This is the first step to revive the auto doc generation (that hasn't been touched for more than 4 years now), so we can get that rolling whenever we land a PR or make a release the docs will be generated and published on ReadTheDocs. The infrastructure is working (i.e. RTD seems to be picking it up), we just need to fix the sphinx build there that's been failing for a long time https://readthedocs.org/projects/python-libjuju/builds/18115299/

I'm not entirely sure yet how to debug the build process on the RTD, however, this change updates and fixes the sphinx setup in the docs, so we can run make html and actually be able to render the docs. The rest is to be figured out.

QA Steps

$ cd docs
$ make html
$ <browser> _build/html/index.html

Notes & Discussion

  • I'm not entirely sure but there might be some local package requirements, which I had to install but I can't remember, I think one of them was python3-sphinxcontrib-asyncio.
  • One of the huge advantages of this is that it also generates a reference for the internal API coming from the facade schema, which allows us (the developers) to see the parameters without going into the depths of the juju/client/. For example, check out the _definitions module here

@jnsgruk
Copy link
Member

jnsgruk commented Jun 3, 2023

@benhoyt recently went through this with ops. Might be some lessons learned there :)

@juanmanuel-tirado
Copy link
Contributor

The changelog is not correctly formatted. I made some changes in this PR cderici#2

R2GO when merged.

@cderici
Copy link
Contributor Author

cderici commented Jun 5, 2023

/merge

@jujubot jujubot merged commit 2596a20 into juju:master Jun 5, 2023
@benhoyt
Copy link
Collaborator

benhoyt commented Jun 6, 2023

@cderici Yeah, I brought my Sphinx config up to date in this recent commit. In particular:

  • locking the version of Sphinx so it was the same locally and on RTD
  • updating to the new format of .readthedocs.yaml
  • lock transitive dependencies (using a script that creates a venv, installs from requirements.in, and then outputs to the locked requirements.txt)

It's worth taking a look at that PR and perhaps doing something similar.

I'd also recommend bringing your Sphinx version up to at least 6.x if possible (I had some issues with the latest, 7.x).

Another thing. It looks like your Makefile make docs command installs the docs requirements into your global Python:

docs: .tox
	$(PIP) install -r docs/requirements.txt
	rm -rf docs/_build/
	$(BIN)/sphinx-build -b html docs/  docs/_build/
	cd docs/_build/ && zip -r docs.zip *

This messes up the dev's machine and makes it harder to iterate, remove deps, etc. I'd strongly recommend using Tox for this (you are already using Tox in this repo, so it should be easy) and adding a tox -e docs instead of using the Makefile. Tox will manage the virtual environment and deps for you.

jujubot added a commit that referenced this pull request Jul 12, 2023
#899

#### Description

This is a follow up on #871 , attempting to have stable green RTD builds for generating the pylibjuju docs.

- It adds a tox environment to run the `sphinx-build` locally rather then doing it in the main dev environment.
- It adds a new `.readthedocs.yaml` configuration file (which will be required it looks like by Sep 2023)

#### Notes & Discussion

After the RTD side is stabilized, the actual docs will be revised to keep up-to-date docs for pylibjuju.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants