-
Notifications
You must be signed in to change notification settings - Fork 121
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
docs: update how-to guides to use the new unit testing framework #1507
docs: update how-to guides to use the new unit testing framework #1507
Conversation
I weirdly get this locally when I build:
It doesn't seem like any changes to the how-to guides should cause that. I'll investigate further. |
I also get it in main, and in Dora's branch that was just merged in with all the new docs. But I'm sure I re-built the docs in that branch before approving the review, and the rtd CI didn't fail. 😕 |
Hmm, maybe @dimaqq already found this? sphinx-doc/sphinx#12715 |
Yes, running tox with an older Python 'fixes' it. |
@tmihoc feel free to review if you have time, but no worries if not :) |
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.
This is massive, thanks, Tony!
PS Thanks also for updating the ops reference links and touching up the language in a lot of other places.
Let of a couple of teeny-tiny comments -- the one at the top about the path for the scenario testing doc is maybe the most relevant at this time.
@@ -108,22 +105,24 @@ def _on_snapshot(self, event: ops.ActionEvent): | |||
... | |||
``` | |||
|
|||
> See more: [`ops.ActionEvent.fail`](https://ops.readthedocs.io/en/latest/#ops.ActionEvent.fail) | |||
> See more: [`ops.ActionEvent.fail`](https://ops.readthedocs.io/en/latest/reference/ops.html#ops.ActionEvent.fail) |
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.
Thanks for updating the reference links! PS Some day I think we should switch to relative paths everywhere. That way moving those reference docs around will no longer break things.
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.
Some day I think we should switch to relative paths everywhere. That way moving those reference docs around will no longer break things.
Agreed. I think we can do better than just relative paths and actually use references, but I'm not sure how to do that in the combination of ReST and myst-markdown. So I opened #1506 so that it's someone else's (maybe future-Tony, maybe not) problem 😆 .
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.
Look good, just a couple of minor (non-blocking) comments
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.
Thanks for doing all of this! - No extra comments from me
Adjustments to the how-to guides:
Live preview