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

Readthedocs and documentation updates #1153

Merged
merged 6 commits into from
Mar 10, 2022
Merged

Readthedocs and documentation updates #1153

merged 6 commits into from
Mar 10, 2022

Conversation

tonyfast
Copy link
Contributor

@tonyfast tonyfast commented Mar 8, 2022

currently readthedocs is failing on builds because qhub isn't installed. this pull request fixes that issue and introduces local documentation build support with nox.

when i make this pull requests, readthedocs should build us some docs or give us errors that hint at fixes.

@tonyfast
Copy link
Contributor Author

tonyfast commented Mar 8, 2022

alrighty, the build is green (https://readthedocs.org/projects/qhub-cloud/builds/16306372/) and can deploy (https://docs.qhub.dev/en/dox-fixes/).

@magsol
Copy link
Contributor

magsol commented Mar 8, 2022

Awesome!! Thanks so much @tonyfast 💪

sphinx-material
sphinxcontrib-bibtex
stringcase
jupyter-book
Copy link
Contributor

@viniciusdc viniciusdc Mar 8, 2022

Choose a reason for hiding this comment

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

Hi @tonyfast, is this enough for building the docs (I know that it worked based on the succeeded build you provided)? I noticed that it has all the sphinx-related dependencies that we might need, but is there anything else we need to add as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

jupyter-book has a ton of dependencies. most of them are to incorporate markdown/notebooks as sphinx documentation source. all those prior deps are taken care of by jupyter-book.

Comment on lines +1 to +8
from nox import session


@session(reuse_venv=True)
def docs(session):
session.install("-e.")
session.install("-rdocs/requirements.txt")
session.run("sphinx-build", "docs", "_build/html")
Copy link
Contributor

@viniciusdc viniciusdc Mar 8, 2022

Choose a reason for hiding this comment

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

I was reading the nox description and it seems very interesting, but I didn't quite understand how it is been used here exactly...

Copy link
Contributor Author

@tonyfast tonyfast Mar 9, 2022

Choose a reason for hiding this comment

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

sorry i missed that description. i added instructions to docs/README.md for building the docs. you'll need nox, pip install nox. run the nox docs session as:

 nox -s docs

nox is a tool to ensure that sphinx builds in its own virtual environment, and doesn't effect the developer's primary environments. its specifically good for something like jupyter-book that has a lot of dependencies that are better isolated from other environments. also, it provides a convenient cli that can work locally/ci.

Copy link
Member

@iameskild iameskild left a comment

Choose a reason for hiding this comment

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

This looks great to me! I ran nox -s docs locally and was able to view the docs (_build/html/index.html).

It seems like we might want to add nox as a dev dependency, what do y'all think?

@tonyfast
Copy link
Contributor Author

tonyfast commented Mar 9, 2022

we could add it as a dev dependency, but that might make the most sense if we can get linting and testing to use nox too.

@tonyfast tonyfast closed this Mar 9, 2022
@tonyfast tonyfast reopened this Mar 9, 2022
@iameskild iameskild merged commit 5a92e2a into main Mar 10, 2022
@iameskild iameskild deleted the dox-fixes branch March 10, 2022 16:43
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.

4 participants