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

docs: go full Diátaxis, ingesting the relevant juju.is/docs/sdk documentation #1481

Merged
merged 16 commits into from
Dec 18, 2024

Conversation

tmihoc
Copy link
Member

@tmihoc tmihoc commented Dec 3, 2024

Preview on ReadTheDocs

This PR adds all the juju.is/docs content featuring Ops.

The content was originally formatted for Discourse; this PR reformats it to serve the RTD project the docs are destined for.

The PR also retitles all how-to guides as "Manage x", often with sections of the form "Implement the feature" and "Test the feature". This extends a pattern we already had in juju docs, which will continue in the juju RTD docs, and which we've also implemented in the new charmcraft RTD docs. Whereas in the past we tended to associate "manage" with a juju admin, here it's used as a catch-all for "how-to do all the things that it is possible to do about x in juju/charmcraft/ops". I believe the pattern is no clumsier than what we had before and having the same terminology across juju/charmcraft/ops , along with cross-referencing, will make it easier for people to piece together the story. (E.g., in Juju, "manage relations" will include juju integrate, while in charmcraft it'll mean declaring a particular endpoint and in ops it'll mean reacting to certain events and using or implementing an interface.)

Finally, the PR trims down the Reference and Explanation docs to just content directly relevant to ops. E.g., the charm taxonomy, events / lifecycle bits, charm maturity, charm best practices are all things that belong at a higher level of abstraction, directly in juju docs. E.g., in the charm best practices doc the first best practice we speak about is using ops. (A lot of that content should also be a lot leaner -- e.g., I'm working to incorporate all the charm SDK event content into the juju doc on hooks, and the charm maturity and charm best practices could really use a complete rethink as well, as they are way too verbose, always understandable, and often duplicate content that's better expressed at a more specific level.)

Some things that I think should be addressed in subsequent PRs:

  • Everywhere: (1) The docs reference juju and charmcraft pages. As those pages are not yet live, the links are empty. Once the pages are live, which should be very soon, the links should be updated. Note: This does create a temporary crisis in the ops RTD docs which, unlike the juju and the charmcraft RTD docs, are actually already live. I believe that's acceptable, especially since (a) at present people still only look at the ops RTD docs for reference and (b) the crisis will be resolved very soon. (2) The testing docs, from the tutorial through the how-to guides and all the way to reference, should be thoroughly revisited to reflect (a) the fact that scenario is now the de facto unit testing tool and that harness is legacy and (b) the fact that most of our "how to manage x" docs have sections on testing"; in short, I anticipate that a lot of that stuff will end up being removed altogether, the result being much leaner and more relevant docs.
  • Tutorial: The Kubernetes tutorial should use charmcraft init (instead of creating all the files by hand) and should only focus on features that are aided by ops (i.e., the publish section should be made part of the planned charmcraft tutorial featuring a regular, non-12-factor app, charm). Also, it's a good idea to make it quite a bit shorter. (When we first created it, it was trying to make up for the fact that our how-to guides were not very helpful. At present, however, I believe it can be much leaner.)
  • How-to guides: The "Manage x" formula, especially as extended just now in the new charmcraft RTD docs, where it includes a "Manage charms", suggest we should perhaps have a "Manage charms" how-to in the ops docs too. If in Juju managing charms means, e.g., juju find, juju download, juju deploy, and juju refresh, and if in charmcraft it means charmcraft init, ... charmcraft pack, charmcraft upload, etc., in ops it might be the place where we give the recipe for the general charm logic from the point of view of ops -- import ops, logging, creating a charm class that inherits from CharmBase, the init method, etc. PS I anticipate some of the content from "Run workloads with..." and the logs how-to might find their home here.

@benhoyt
Copy link
Collaborator

benhoyt commented Dec 3, 2024

Thanks @tmihoc -- that's a big change! We'll discuss how to review and proceed with this in our Charm Tech daily sync today and let you know what we decide.

There are many 'cross-reference target not found' warnings where there is no xref - these are all to content that isn't yet published - see the PR description for details.

There is still a huge number of warnings in the earlier stages - I'll do a separate commit for those.
@benhoyt
Copy link
Collaborator

benhoyt commented Dec 4, 2024

Thanks again for your work on this, @tmihoc. @dwilding is going to work with you from here to drive this forward. We discussed some details briefly today in our daily sync. We're very happy this is moving to RTD, we're mostly happy with the structure. We have a few things we'd like addressed before merging this PR, that David will work with you on:

  • We'd like to link to the old Juju/Charmcraft stuff for now, so we don't have broken links right away. Once Juju/Charmcraft is moved, we can open a PR and quickly merge that to fix.
  • Fix the “Ops (ops)” stuttering and on the home page and have a better title.
  • Move Ops Python reference to the second level (directly under Reference), move other content elsewhere.

Copy link
Contributor

@tonyandrewmeyer tonyandrewmeyer left a comment

Choose a reason for hiding this comment

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

Adding a few comments I did before the team discussion on this.

docs/index.md Outdated Show resolved Hide resolved
docs/howto/index.md Show resolved Hide resolved
docs/explanation/index.md Show resolved Hide resolved
docs/reference/index.md Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/custom_conf.py Outdated Show resolved Hide resolved
docs/howto/instrument-your-charm-with-tracing-telemetry.md Outdated Show resolved Hide resolved
docs/reference/pytest-operator.md Outdated Show resolved Hide resolved
@tmihoc
Copy link
Member Author

tmihoc commented Dec 17, 2024

My commit just now does all of the following:

  • comments out empty Juju and Chamrcraft links (using <!-- UPDATE LINKS -->, so we can easily find the links to update later)
  • re-enables myst.xref_missing. See note 1.
  • updates the homepage intro as agreed with David. See note 2.
  • updates the homepage community chat.
  • removes “How to instrument your charm with tracing telemetry" from the how-to guides. This is not a doc we should keep in Ops docs.
  • updates the reference, moving pytest-operator to Testing > Integration testing and moving all of the non-ops docs to explanation. See note 3.

Note 1: There is still this warning but I'm not sure how to fix it:

.../operator/.tox/docs-live/lib/python3.12/site-packages/sphinx_tabs/tabs.py:335: RemovedInSphinx90Warning: The str interface for _JavaScript objects is deprecated. Use js.filename instead.
  if path.suffix == ".js" and path.as_posix() in context["script_files"]:
.../operator/.tox/docs-live/lib/python3.12/site-packages/sphinx_tabs/tabs.py:336: RemovedInSphinx90Warning: The str interface for _JavaScript objects is deprecated. Use js.filename instead.
  context["script_files"].remove(path.as_posix())

Note 2: In the homepage intro, removing the last line means that the intro is no longer compliant with the standard homepage template. In a future iteration we should find a way to comment on "who the product is useful for".

Note 3: While I agree that in principle those topics are not Ops reference, they're not Ops explanation either. Also, while I understand why we want to promote the various ops modules so they're right under reference, I think that pattern may be too strict for ops reference needs overall. Something to consider in the future.

@dwilding
Copy link
Contributor

@tmihoc Thank you very much for putting all of this together! I will review in detail in my morning

It looks like the RTD build is failing because of a few more missing xrefs:

[...] /howto/manage-libraries.md:214: WARNING: 'myst' cross-reference target not found: '' [myst.xref_missing]
[...] /howto/run-workloads-with-a-charm-kubernetes.md:8: WARNING: 'myst' cross-reference target not found: '' [myst.xref_missing]
[...] /howto/write-integration-tests-for-a-charm.md:84: WARNING: 'myst' cross-reference target not found: 'run-tests' [myst.xref_missing]

@tonyandrewmeyer
Copy link
Contributor

Thanks so much for all the work on this!

Note 1: There is still this warning but I'm not sure how to fix it:

.../operator/.tox/docs-live/lib/python3.12/site-packages/sphinx_tabs/tabs.py:335: RemovedInSphinx90Warning: The str interface for _JavaScript objects is deprecated. Use js.filename instead.
  if path.suffix == ".js" and path.as_posix() in context["script_files"]:
.../operator/.tox/docs-live/lib/python3.12/site-packages/sphinx_tabs/tabs.py:336: RemovedInSphinx90Warning: The str interface for _JavaScript objects is deprecated. Use js.filename instead.
  context["script_files"].remove(path.as_posix())

That's also in main, and it doesn't cause the build to fail. I believe at some point we/the starter pack will update Sphinx and it will go away.

Note 3: While I agree that in principle those topics are not Ops reference, they're not Ops explanation either. Also, while I understand why we want to promote the various ops modules so they're right under reference, I think that pattern may be too strict for ops reference needs overall. Something to consider in the future.

My feeling is that there's a bit of tension between ops.rtd being "documentation for writing and building charms" and "documentation for the Python ops package". If it's the latter then I do think all the reference can fit into the various modules, whether it's module-level or class-/function-level. If it's the former, then I do think some adjustment may be needed later. I think once all the pieces of the new Juju doc ecosystem are in place and we've all done some work on them and lived in it for a while, it'll be clearer.

I'll do a review in the morning.

@tmihoc tmihoc force-pushed the add-discourse-docs branch from b53d96a to 9454b1c Compare December 17, 2024 10:37
@tmihoc
Copy link
Member Author

tmihoc commented Dec 17, 2024

It looks like the RTD build is failing because of a few more missing xrefs:

[...] /howto/manage-libraries.md:214: WARNING: 'myst' cross-reference target not found: '' [myst.xref_missing]
[...] /howto/run-workloads-with-a-charm-kubernetes.md:8: WARNING: 'myst' cross-reference target not found: '' [myst.xref_missing]
[...] /howto/write-integration-tests-for-a-charm.md:84: WARNING: 'myst' cross-reference target not found: 'run-tests' [myst.xref_missing]

Fixed, thanks

@tmihoc
Copy link
Member Author

tmihoc commented Dec 17, 2024

Note 3: While I agree that in principle those topics are not Ops reference, they're not Ops explanation either. Also, while I understand why we want to promote the various ops modules so they're right under reference, I think that pattern may be too strict for ops reference needs overall. Something to consider in the future.

My feeling is that there's a bit of tension between ops.rtd being "documentation for writing and building charms" and "documentation for the Python ops package". If it's the latter then I do think all the reference can fit into the various modules, whether it's module-level or class-/function-level. If it's the former, then I do think some adjustment may be needed later. I think once all the pieces of the new Juju doc ecosystem are in place and we've all done some work on them and lived in it for a while, it'll be clearer.

On the federated docs paradigm, these are Ops docs. Of course, Ops docs are about developing and testing charms, so it's about that as well. That said, stuff like pytest-operator and charm-relation-interfaces are in here only because we don't have a better place for them.

Copy link
Contributor

@tonyandrewmeyer tonyandrewmeyer left a comment

Choose a reason for hiding this comment

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

Thanks again for all the work on this! I'm really looking forward to the step-change in docs maintenance that I feel this will enable.

Note that I have not really reviewed any of the actual documentation content (other than a bit of reading while I was addressing warnings) - only the top level structure and the landing pages. I'm assuming it's a fairly faithful export from Discourse and anything that needs correction can be done later.

A couple of small comments/suggestions, but I'm happy for this to be merged. If there are remaining issues it'll be easy to quickly make adjustments in later PRs.

Comment on lines +86 to +87
* **[Join our online chat](https://matrix.to/#/#charmhub-ops:ubuntu.com)**:
Meet us in the #charmhub-charmdev channel on Matrix.
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL that the link to the "Ops Library Development" room is "#charmhub-ops". I probably caused this change with an error comment, sorry, but we're now inconsistent.

Note that the footer (from docs/custom_conf.py) links to the Charm Development channel. It's a bit odd having "Join our online chat" on the landing page be to Ops Library Development and the footer on every page having an "Ask a question on Matrix" link that goes to Charm Development.

My feeling is that most of the time we'd get questions about writing and testing charms, so Charm Development is the best place. If people do have a question about developing and maintaining ops itself and they posted in Charm Development, I don't think we'd miss it, and it'd be less likely.

So my suggestion is:

Suggested change
* **[Join our online chat](https://matrix.to/#/#charmhub-ops:ubuntu.com)**:
Meet us in the #charmhub-charmdev channel on Matrix.
* **[Join our online chat](https://matrix.to/#/#charmhub-charmdev:ubuntu.com)**:
Meet us in the Charm Development (#charmhub-charmdev) channel on Matrix.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can do this in a follow-up PR

Comment on lines +88 to +89
* **[Report bugs](https://github.com/canonical/operator/)**:
We want to know about the problems so we can fix them.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker in any way, but it still seems that /issues would be the more logical place for a "report bugs" link.

Comment on lines +92 to +93
* **[Contribute docs](https://github.com/canonical/operator/tree/main/docs)**:
The documentation sources on GitHub.
Copy link
Contributor

Choose a reason for hiding this comment

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

@dwilding as a follow-up job to this PR, what do you think about adding a README.md file in the docs folder that explains how to contribute docs? With this change, we're basically just dropping them in a folder, which isn't super friendly.

We could move the HACKING.md content (leaving a link there) and probably improve it a bunch, particularly now that everything is in git rather than Discourse (some inspiration could be taken from freesewing maybe!).

I don't think this would be part of the published docs, just something to show up when you navigate to the docs folder in GitHub or in a clone.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice idea - I can work on this in a follow-up PR. Also, I'm looking at "Edit this page on GitHub" in the footer. We don't really want people to directly submit edits, right? We'd rather that people follow the process explained in HACKING.md or the new docs README.md

Copy link
Member Author

Choose a reason for hiding this comment

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

If I may chime in: Imo the README and the CONTRIBUTING are repo-level thingies. As in, I wouldn't nest them under the docs. At the same time, they are a minimal manifestation of user docs and, respectively, contributor docs, which I would nest under docs for every project. Here's the incarnation of this idea for Juju: https://github.com/juju/juju/tree/3.6/docs (note the "user" and "contributor" directories and the top-level https://github.com/juju/juju/blob/3.6/docs/index.md that introduces both.

docs/index.md Outdated Show resolved Hide resolved
@tonyandrewmeyer tonyandrewmeyer changed the title docs: add and reformat discourse docs docs: ingest the relevant juju.is/docs/sdk documentation, going full Diátaxis Dec 18, 2024
@tonyandrewmeyer tonyandrewmeyer changed the title docs: ingest the relevant juju.is/docs/sdk documentation, going full Diátaxis docs: go full Diátaxis, ingesting the relevant juju.is/docs/sdk documentation Dec 18, 2024
@tonyandrewmeyer tonyandrewmeyer merged commit d2508f8 into canonical:main Dec 18, 2024
34 checks passed
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