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

📚 Add RTD documentation #23

Merged
merged 10 commits into from
Jun 11, 2021
Merged

Conversation

chrisjsewell
Copy link
Contributor

@chrisjsewell chrisjsewell commented Aug 31, 2020

The simplest implementation of RTD documentation!

Simply run tox -e docs-clean to test, and tox -e docs-update to try rebuilding
(obviously a maintainer will need to add it on RTD)

In doing this, I realised that the rebuild fix (#19) wasn't actually working properly.
This is because the sphinx env does not get re-saved after build-finished events.

So instead you'll see that I now read/write a JSON file in the build folder. This is also easier to inspect manually 😄

The simplest implementation of RTD documentation!
@Daltz333
Copy link
Member

I'd like the rebuild fix in a separate PR, as it's an organization policy for all merges to be squashed.

@chrisjsewell
Copy link
Contributor Author

strange policy, fair enough

@Daltz333
Copy link
Member

Daltz333 commented Aug 31, 2020

It's to enforce separation of concerns, as the primary contributors to the projects are usually external. Plus merge commits can get messy if the contributor isn't all that familiar with git, which is common in our case.

@chrisjsewell
Copy link
Contributor Author

done 👍

@chrisjsewell chrisjsewell changed the title 📚 Add RTD documentation (+rebuild fix) 📚 Add RTD documentation Aug 31, 2020
Copy link
Member

@Daltz333 Daltz333 left a comment

Choose a reason for hiding this comment

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

I think it would be better to keep the documentation in rST format, as our main project https://github.com/wpilibsuite/frc-docs uses it extensively (the reason we wrote all these extensions :) and possibly more). However, I don't feel like we should rewrite the README especially with how simple it is. I'll okay using myst.

However, I'd like to keep things consistent with our other repositories and using sphinx_rtd_theme with the logo using the rediraffe logo. Additionally, using sphinx-build has had an iffy chance working on windows (due to scripts sometimes not being executable), so could you add in the make.bat and make files that are generated when you run the sphinx-quickstart command.

I'd like to adopt our documentation style guide into this repository and eventually add our other CI checks for the documentation. See here for our style guide.

docs/conf.py Outdated
master_doc = "index"
project = "sphinxext-rediraffe"

exclude_patterns = ["_build", "Thumbs.db", ".DS_Store"]
Copy link
Member

Choose a reason for hiding this comment

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

I don't think excluding anything but _build is necessary, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeh this is what we use in our excutablebooks packages, but just _build should generally be fine.

@chrisjsewell
Copy link
Contributor Author

chrisjsewell commented Sep 2, 2020

the reason we wrote all these extensions :) and possibly more

You know that myst supports all these extensions (it supports "every" sphinx extension)? I will be adding myst to the sphinx-tabs documentation at some point.
Happy to help you convert to myst 😆 (there's a package in the works 😉 https://github.com/mmcky/sphinxcontrib-rst2myst)

But enough of the sales pitch lol, I implemented your review changes

Additionally, using sphinx-build has had an iffy chance working on windows (due to scripts sometimes not being executable)

Out of interest, you've had this issue recently? I don't use Windows (because its rubbish 😛) so I don't know personally, but I haven't heard of anyone having an issue with python console_script entry points.
I always felt this was a bit of a legacy thing in sphinx, left over from python2.

I'd like to adopt our documentation style guide into this repository and eventually add our other CI checks for the documentation. See here for our style guide.

I would suggest creating a .github repository for this organisation, containing this information, which then gets linked in issues/PRs across all repositories, see https://github.com/executablebooks/.github

FYI you can now have readthedocs directly run CI: https://docs.readthedocs.io/en/stable/guides/autobuild-docs-for-pull-requests.html

Also, fancy releasing a v0.2.2 soon. I've already had people complaining about rebuilds failing for jupyter-book docs 😬

@Daltz333
Copy link
Member

Daltz333 commented Sep 2, 2020

It's been pushed. And I'm aware of RTD CI, as we do use it. However, it's very difficult to replicate the RTD build environment exactly, so our CI exists to replicate a "normal" build. Additionally it tests other things such as spelling, image size, verifying all links are valid, valid redirects (this extension).

@chrisjsewell
Copy link
Contributor Author

it tests other things such as spelling, image size, verifying all links are valid

Fair, I'll have to check it out some time 👍

@chrisjsewell
Copy link
Contributor Author

It's been pushed.

thanks 😄

Base automatically changed from master to main January 31, 2021 03:41
@AustinShalit
Copy link
Member

@Daltz333 status update on this?

@TheTripleV
Copy link
Member

The theme should be switched to furo to match sphinxext-opengraph.
Then, after the rtd project is created, this should be be good to go.

@TheTripleV TheTripleV merged commit d8961c2 into wpilibsuite:main Jun 11, 2021
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.

None yet

4 participants