-
Notifications
You must be signed in to change notification settings - Fork 73
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: add, refactor, and reformat discourse docs feat. charmcraft #2010
base: main
Are you sure you want to change the base?
Conversation
We can ignore the test failures, but the linter that's failing is codespell and is related to these changes: https://github.com/canonical/charmcraft/actions/runs/12084317065/job/33699249108?pr=2010#step:6:637 |
b86dd66
to
b69eb9d
Compare
I think I've addressed all of these now. |
docs/tutorial/write-your-first-kubernetes-charm-for-a-django-app.md
Outdated
Show resolved
Hide resolved
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.
Looks good, thanks!
Hey, so |
The following pages have extensive maintenance planned post-migraiton, so they don't need detailed review:
|
I'm breaking down the content from PR #2010 into smaller chunks. This is "How to manage 12-factor app charms" converted to RST: https://github.com/canonical/charmcraft/blob/737976c943a601adccd883ffa6a276bce625c9a1/docs/howto/manage-12-factor-app-charms.md The main differences from the Markdown version: - Uses tabs instead of dropdowns (matching the Rockcraft docs: https://documentation.ubuntu.com/rockcraft/en/latest/how-to/build-a-12-factor-app-rock/) - "See also" blocks are added as cards
I'm breaking down the content from PR #2010 into smaller chunks. This is "How to manage 12-factor app charms" converted to RST: https://github.com/canonical/charmcraft/blob/737976c943a601adccd883ffa6a276bce625c9a1/docs/howto/manage-12-factor-app-charms.md The main differences from the Markdown version: - Uses tabs instead of dropdowns (matching the Rockcraft docs: https://documentation.ubuntu.com/rockcraft/en/latest/how-to/build-a-12-factor-app-rock/) - "See also" blocks are added as cards
I'm breaking down the content from PR #2010 into smaller chunks. This is "How to manage 12-factor app charms" converted to RST: https://github.com/canonical/charmcraft/blob/737976c943a601adccd883ffa6a276bce625c9a1/docs/howto/manage-12-factor-app-charms.md The main differences from the Markdown version: - Uses tabs instead of dropdowns (matching the Rockcraft docs: https://documentation.ubuntu.com/rockcraft/en/latest/how-to/build-a-12-factor-app-rock/) - "See also" blocks are added as cards
This reorganises the part reference in line with #2010
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.
Approving without review. If we merge this, we can resolve any code issues during separate RST conversion tasks.
I'm breaking down the content from PR #2010 into smaller chunks. This is "How to manage 12-factor app charms" converted to RST: https://github.com/canonical/charmcraft/blob/737976c943a601adccd883ffa6a276bce625c9a1/docs/howto/manage-12-factor-app-charms.md The main differences from the Markdown version: - Uses tabs instead of dropdowns (matching the Rockcraft docs: https://documentation.ubuntu.com/rockcraft/en/latest/how-to/build-a-12-factor-app-rock/) - "See also" blocks are added as cards
The current Flask tutorial: https://juju.is/docs/sdk/write-your-first-kubernetes-charm-for-a-flask-app This PR will move the tutorial off of Discourse and Charm SDK and onto the Charmcraft RTD. --------- Co-authored-by: Michael DuBelko <[email protected]> Co-authored-by: Alex Lowe <[email protected]>
docs/tutorial/index.rst
Outdated
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.
@medubelko please look at this file
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 one is encroaching on CRAFT-3702, therefore I don't think we need to exercise a huge amount of discretion at this point. I've sent you the landing page exemplar we use.
We've decided to merge this this week, with the caveat that we will make changes to the structure which has the potential to break links |
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 left comments for the files you requested.
@@ -0,0 +1,901 @@ | |||
.. _file-charmcraft-yaml: | |||
.. highlight:: yaml |
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.
Since highlight
acts as a switch, I think the code would be easier to read if it were placed immediately before the first block that uses it.
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's pretty unintuitive to me - if I'm looking for why a block is highlighted in an unexpected manner, I'm going to check that block (which is likely not the first block in the page, especially for reference pages like this), then the directives at the very top of the page, then the global highlight_language
configuration.
A highlight
directive somewhere in the middle of a file would be highly unappreciated, much like an import
coming between two class definitions in a source file. Worse still would be a switch of the highlight
directive mid-file, which would be a form of code smell that this document is about two different things (and thus should be broken up into multiple pages), since in cases where one needs to change the highlight language a code-block
can set its own language as a one-off.
.. highlight:: yaml | ||
|
||
``charmcraft.yaml`` | ||
******************* |
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.
A nitpick, but I've been trying to stick to the heading conventions recommended by the style guide.
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 - yeah, @tmihoc pointed this out to me yesterday too, which was actually my first knowledge about us having an rST style guide specifically.
The full list of keys is defined in the Charmcraft project (but this implies upstream keys from craft-application): | ||
https://github.com/canonical/charmcraft/blob/3.2.0/charmcraft/models/project.py#L381-L1070 | ||
|
||
Technically, the only key required upfront is type. But then, depending on what you choose, other keys become required as well. (The required keys are the ones that are set to a value, unless that value is a pydantic.Field that doesn't have either a default or a default_factory parameter) |
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.
We've been very consistent about 2 spaces for indentation. You may recall we recently discussed switching my YAML examples in Snapcraft back to 2 spaces.
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.
The current configuration for markdown and rst in starbase is the global default of indent_size = 4
, so if we want to change that well need to change the standard and the existing documentation files, including in Starbase itself.
The indent size of 2 for YAML files has been in Starbase for 22 months.
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've created an issue on Starbase so we can get clarification for everything that needs changing here.
docs/tutorial/index.rst
Outdated
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 one is encroaching on CRAFT-3702, therefore I don't think we need to exercise a huge amount of discretion at this point. I've sent you the landing page exemplar we use.
Co-authored-by: Michael DuBelko <[email protected]>
This PR adds all the juju.is/docs content featuring Charmcraft.
The content was originally formatted for Discourse; this PR reformats it to serve the RTD project the docs are destined for.
The content was also originally entangled with Ops content; this PR significantly refactors -- as well as clarifies and tries to complete -- everything to serve the stand-alone Charmcraft docs set it is destined for.
Issues that should be fixed in subsequent PRs:
analyse
andtest
are not featured in any how-to guide.charmcraft
CLI command reference docs should have anchors on the templatecommand-charmcraft-x
and titles on the template "Commandcharmcraft x
. This will not better align with existing practices in the Juju world but also results in more clarity when linking to the page using{ref}
...`` (2) The various file reference docs should be as much as possible autogenerated from source (likely the source will have to be updated first) -- at the very least, filecharmcraft.yaml
. They may also need further clean-up. (3) The part docs are a bit of a mess. We should move any relevant content to the part docs, then link up to that and only keep in Charmcraft the material specific to Charmcraft docs, and then revisit that material as well to streamline it as much as possible (e.g., currently the dump plugin is documented in 3 places). (4) Charmcraft analyzers and linters -- this gives details on pack and analyse but it's not clear in what way it's useful to the reader, so we need to rethink.PS Some docs may have a Contributors list on the bottom, listing the Discourse handles of the contributors to date. We'll want to rethink how we do that.