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: fill out remaining external link placeholders #1564

Merged

Conversation

tonyandrewmeyer
Copy link
Contributor

All the remaining "UPDATE LINKS" notes in the docs are replaced with actual links to the external Juju/Charmcraft docs.

Also changes the "product page" to be juju.is rather than juju.is/docs/sdk as the latter is going away (juju.is was recommended by @tmihoc).

(These changes were previously in a commit in another PR but got lost when changes were forced - it seemed cleanest to break them out into a separate PR).

Comment on lines -380 to -383
<!-- UPDATE LINKS:
> Example implementations: [`mysql-k8s-operator`]()
-->

Copy link
Contributor

Choose a reason for hiding this comment

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

Dropping this seems fine especially since we're going to be recommending jubilant for integration tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I figure this entire page will be rewritten this cycle so there's no much point making any improvements.

@dwilding
Copy link
Contributor

Thanks @tonyandrewmeyer & @tmihoc. Approving with a couple of observations, which I can revisit (please add any thoughts that you have):

  • I believe that the official syntax for an intersphinx link in MyST is:

    [Juju Storage](inv:juju:std:label#manage-storage)
    

    instead of:

    {external+juju:ref}`Juju Storage <manage-storage>`
    

    I don't think there's any difference in practice, though. So I'm in two minds about whether it's worth changing all the links.

  • Out of the scope of this PR, but we still have a bunch of hard-coded links to juju.is, some of which might be broken. For instance, the link to "Set up / tear down automatically" from Set up your development environment is broken. I can review and migrate these links in a separate PR.

@tmihoc
Copy link
Member

tmihoc commented Feb 10, 2025

@dwilding I was just hacking the syntax from rST (and was in fact not aware of the possibility you mention). That said, I do think the brackets syntax suggests external link whereas the one I used aligns more with the {ref} syntax for internal links. As the point of intersphinx is to make external links local, I'd favor the syntax that better suggests that. That said, I really don't have strong feelings about this.

@dwilding
Copy link
Contributor

I like your reasoning, @tmihoc. FWIW, in MyST .md files I prefer [Link title](#reference) instead of the {ref} syntax, so to me the brackets do look more like internal links 😄

Anyway, again, no strong feelings. @tonyandrewmeyer I don't have any objection to merging this as-is.

@tonyandrewmeyer tonyandrewmeyer merged commit 0b23b6f into canonical:main Feb 11, 2025
32 checks passed
@tonyandrewmeyer tonyandrewmeyer deleted the fill-missing-doc-links branch February 11, 2025 05:54
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