-
-
Notifications
You must be signed in to change notification settings - Fork 419
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
base: main
Are you sure you want to change the base?
Conversation
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>
This reverts commit 12b4654.
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
…t to true Signed-off-by: Ivan Schaller <ivan@schaller.sh>
There was a problem hiding this 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/ |
There was a problem hiding this comment.
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") |
There was a problem hiding this comment.
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.)
Line 3 in c2be2bc
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Liking the progress
.github/workflows/docs.yml
Outdated
- "v*.*.*" | ||
|
||
pull_request: | ||
branches: [main, master] |
There was a problem hiding this comment.
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
branches: [main, master] | |
branches: [main] |
zipp==3.20.2 | ||
rich==14.0.0 | ||
ruamel.yaml.clib==0.2.12 | ||
ruamel.yaml==0.18.14 |
There was a problem hiding this comment.
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>
And for that i would like to know what route you want to continue on, either:
For both routes you would need to decide some points like:
|
Not 100% sure what you mean by "formatter" settings here. There's already some black settings in there.
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.
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. |
For ruff there are more settings, but as black works fine, only the option |
Had to make some fairly minor changes to the logic of the update-requirement scripts across all the repos to deal with |
Signed-off-by: Ivan Schaller <ivan@schaller.sh>
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 |
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
There are also two important Sphinx extensions in use:
Build documentation
Manual
script/get-docs
to gather all modules from octodns and create the.rst
files which Sphinx needs.script/build-docs
to build the documentation. Output will be on the pathdocs/_build/html
docs/_build/html/index.html
GitHub Action
A GitHub actions workflow was also created to build on PR, Tag and manual trigger.
gh-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
Best regards