8000 DRAFT: Add Sphinx for Documentation by olofvndrhr · Pull Request #1261 · octodns/octodns · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

DRAFT: Add Sphinx for Documentation #1261

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 communi 8000 ty.

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

Draft
wants to merge 27 commits into
base: main
Choose a base branch
from
Draft

Conversation

olofvndrhr
Copy link
Contributor

Here is the first iteration of the added Documentation via Sphinx.
I played with it a bit and it looks promising. This is of course not "final" but a prototype.
Themes, settings etc can be tweaked.

Live preview

To see how everything looks, check: https://olofvndrhr.github.io/octodns/
It will be rebuilt on all changes of this PR, so everyone can see how it then looks.

Added/changed

Automatic Documentation via Sphinx Autodocs

Documentation file structure

octodns/
└── docs/
    ├── _build/      -> generated html (are git ignored)
    ├── modules/     -> automatically generated module files (are git ignored)
    ├── _static/     -> static content like files, icons etc.
    ├── infos/       -> special pages with project infos (license, changelog etc.)
    └── pages/       -> manual documentation about topics/guides

There are also two important Sphinx extensions in use:

  • sphinx.ext.napoleon -> which makes it possible to use the NumPy oder Google Docstring style
  • myst_parser -> which makes it possible to write doc 10000 umentation in Markdown format instead of ReStructuredText

Build documentation

Manual

  1. Run the script script/get-docs to gather all modules from octodns and create the .rst files which Sphinx needs.
  2. Run the script script/build-docs to build the documentation. Output will be on the path docs/_build/html
  3. Check the generated docs -> open docs/_build/html/index.html

The generated modules are in the .gitignore as they would just spam the repo with files

GitHub Action

A GitHub actions workflow was also created to build on PR, Tag and manual trigger.

  1. Installs python and all dependencies from requirements-dev.txt
  2. Installs the project (octodns) locally
  3. Gathers the modules and builds the documentation
  4. Pushes all built documentation (the html) to the branch gh-pages
  5. (if configured) The GitHub bot deploys the html to GitHub Pages

Automatic Markdown formatting

As all new documentation will be written in Markdown, there needs to be a standard to follow.
As i already saw on my previous PR, auto-formatters will change the formatting if not careful.

So i added mdformat to the workflow.
Why mdformat? Because it has no dependencies and is also written in python (instead of something like prettier).

I adapted the scripts script/format to include markdown files and ran it also. Thats why so many markdown files are changed in the PR.

Other

  • The requirements were updated for the new dependencies
  • Some Docstrings were updated for the new format
  • The ACME processor was updated with new documentation (as a prototype to see what's possible and how it looks)

Best regards

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
…ild by ci

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
… style in the whole project. also needed for further documentation as its written in markdown.

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
This reverts commit d84b85a.
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
…t to true

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Copy link
Contributor
@ross ross left a comment

Choose a reason for hiding this comment

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

That looks really good. My first question was whether the examples stuff would be included and I'm glad to see that it is.

I adapted the scripts script/format to include markdown files and ran it also. Thats why so many markdown files are changed in the PR.

See https://github.com/octodns/octodns/blob/main/.git-blame-ignore-revs for previous times when large formatting-specific changes were made. That file allows us to tell git/GitHub to ignore those revs when looking at the history of files so that don't show up as noisy changes. If all the formatting-specific changes happen in a standalone commit we can add its SHA to the list w/a comment about it.

The requirements were updated for the new dependencies

Would it make sense to have a docs specific extras_require and then modify ./script/update-requirements to create requirements-docs.txt? Don't have an opinion really, just offering it up as a option.

The ACME processor was updated with new documentation (as a prototype to see what's possible and how it looks)

Nice. Having a destination for the docs will definitely make me more likely to go through and clean up and add new stuff.

.gitignore Outdated
@@ -17,3 +17,5 @@ octodns.egg-info/
output/
tests/zones/unit.tests.
tmp/
docs/_build/
docs/modules/
Copy link
Contributor

Choose a reason for hiding this comment

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

alphabetize if you don't mind. I always use !sort in vim so the existing order should likely be from that.

@@ -8,36 +8,61 @@


class AcmeManagingProcessor(BaseProcessor):
log = getLogger('AcmeManagingProcessor')
log = getLogger("AcmeManagingProcessor")
Copy link
Contributor

Choose a reason for hiding this comment

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

What made all the quote changes from ' to "?

octoDNS's pyproject.toml tells black to leave string quoting alone as the single quote variant is used as the standard across all of the octoDNS modules (predating black's usage.)

skip-string-normalization=true

It's a lot of churn here and would be across all the modules. Wouldn't be the end of the world to change it, but I think I'd prefer to do it in a dedicated PR if/when that happens since it isn't directly related to the documentation. We'd also need to make sure that the black settings change too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I run ruff as an auto-formatter in my IDE, and the default there is double-quotes. That's why its changed on all files where i modified the Docstring.
Will change it back.
And before the real PR is opened i will also squash the commits, so its not a mess of commits.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shame it doesn't use the black settings in pyproject.toml since it's aiming to replace it and other related tooling.

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
…s to also generate them

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
@olofvndrhr
Copy link
Contributor Author
  • Added now the new extra dependencies for "docs" and updated the script to generate them.
    Made it a simple loop, as not to copy+paste the same code 3 times.

  • Gitignore is also now sorted.

And i checked all source files about their quote style. And unfortunatly its not consistent at all. So i think its needed to add a specific "style guide" and the formatter settings to make it the same across the project.

@ross
Copy link
Contributor
ross commented Jun 17, 2025

And i checked all source files about their quote style. And unfortunatly its not consistent at all. So i think its needed to add a specific "style guide" and the formatter settings to make it the same across the project.

Yeah. black only offered a "leave quotes alone" option so I went with that. There have definitely been things slip through PR review and stuff around quoting.

Copy link
Contributor
@ross ross left a comment

Choose a reason for hiding this comment

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

Liking the progress

- "v*.*.*"

pull_request:
branches: [main, master]
Copy link
Contributor

Choose a reason for hiding this comment

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

all the octoDNS repos use main

Suggested change
branches: [main, master]
branches: [main]

zipp==3.20.2
rich==14.0.0
ruamel.yaml.clib==0.2.12
ruamel.yaml==0.18.14
Copy link
Contributor

Choose a reason for hiding this comment

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

What OS and python are you working on? Seeing a bit more change here than I'd expect from an update and wondering if it's different requirements on different envs.

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
@olofvndrhr
Copy link
Contributor Author

Yeah. black only offered a "leave quotes alone" option so I went with that. There have definitely been things slip through PR review and stuff around quoting.

And for that i would like to know what route you want to continue on, either:

  • still use black and isort, but add formatter settings to pyproject.toml
  • switch to ruff as it already includes both and has some nice stuff that "fixes issues"

For both routes you would need to decide some points like:

  • quote settings for strings
  • quote settings for docstrings
  • line length (still 80?)
  • docstring style (google/numpy)

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
@ross
Copy link
Contributor
ross commented Jun 19, 2025

And for that i would like to know what route you want to continue on, either:

* still use `black` and `isort`, but add formatter settings to `pyproject.toml`

Not 100% sure what you mean by "formatter" settings here. There's already some black settings in there.

* switch to `ruff` as it already includes both and has some nice stuff that "fixes issues"

I've been fine with the current setup. If there's non-trivial "fixes issues" to ruff I'd consider it...

At a high level the documentation PR isn't the places to make general formatting changes or changes to the formatting tooling. With the one change pr PR mantra in mind I'd prefer to keep this focused on the documentation generation and any changes directly related to making that work and/or have a better result.

For both routes you would need to decide some points like:
...

Yeah, this would all be a separate PR and discussion. If those non-functional formatting changes happen then they can be done there in a dedicated commit(s) and added to the blame ignores file the way the original black changes were.

@olofvndrhr
Copy link
Contributor Author

Yeah, this would all be a separate PR and discussion. If those non-functional formatting changes happen then they can be done there in a dedicated commit(s) and added to the blame ignores file the way the original black changes were.

Yea thats true. For me in this PR only the Docstring style is important, as i'm updating some for Sphinx.

@olofvndrhr
Copy link
Contributor Author

Not 100% sure what you mean by "formatter" settings here. There's already some black settings in there.

For ruff there are more settings, but as black works fine, only the option skip-string-normalization=true would need to be removed to unify all quotes.
But thats something for another PR, you're right about that

@ross
Copy link
Contributor
ross commented Jun 25, 2025

script/update-requirements

Had to make some fairly minor changes to the logic of the update-requirement scripts across all the repos to deal with click deprecating support for python 3.9 early. Should be fairly straightforward to add that logic to the update version in here. LMK if you want me to take a poke at it.

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
@olofvndrhr
Copy link
Contributor Author

script/update-requirements

Had to make some fairly minor changes to the logic of the update-requirement scripts across all the repos to deal with click deprecating support for python 3.9 early. Should be fairly straightforward to add that logic to the update version in here. LMK if you want me to take a poke at it.

Updated the script. Now also works on my version as it doesn't always "find" click.

    i = [i for i, r in enumerate(output) if r.startswith('click==')][0]
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^
IndexError: list index out of range

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.

2 participants
0