docs: give each best-practice admonition a stable :name: anchor

.github/update-best-practice-table.py

@@ -22,6 +22,11 @@
 associated with them) and produce a suitable block of Markdown to include in the
 Ops documentation.
 
+Each ``Best practice`` admonition may declare a ``:name:`` option, which is used
+as a stable identifier for the practice. The generated output tags each bullet
+with a trailing ``<!-- id: <name> -->`` HTML comment, so downstream automation
+can match practices by ID.
+
 ## Local Usage
 
 You'll need a clone of the `operator` repository (presumably the one you're running this command
@@ -41,25 +46,51 @@
 import pathlib
 import re
 
+# Block at the top of the generated file, explaining how downstream automation
+# can locate per-practice identifiers. The body cannot include a literal `-->`
+# (it would close the surrounding HTML comment early and leak the remainder of
+# the header into the rendered page), so the example syntax is described
+# without showing the closing characters.
+HEADER = """\
+<!--
+This file is auto-generated by .github/update-best-practice-table.py from
+"Best practice" admonitions in the operator and charmcraft documentation.
+
+Each practice that declares a :name: slug in its source admonition has its
+bullet tagged with a trailing HTML id-comment containing the slug so that
+downstream automation (e.g. charmhub-listing-review) can match practices
+by ID.
+
+Do not edit this file directly; update the source admonition instead.
+-->
+"""
+
 
 def extract_best_practice_blocks_rst(file_path: pathlib.Path, content: str):
     """Extracts 'Best practice' blocks from a ReST file.
 
-    Returns a tuple of (heading, reference, content).
+    Returns a list of (heading, reference, content, name) tuples. ``name`` is
+    the value of the admonition's ``:name:`` option, or ``None`` if not set.
     """
     lines = content.splitlines()
-    results: list[tuple[str | None, str | None, str]] = []
+    results: list[tuple[str | None, str | None, str, str | None]] = []
 
     current_heading = None
     current_ref = None
     previous_line = ''
     inside_admonition = False
     admonition_lines: list[str] = []
+    admonition_name: str | None = None
 
     for line in lines:
         if line.strip() == ':class: hint':
             continue
 
+        name_match = inside_admonition and re.match(r'\s*:name:\s*(\S+)\s*$', line)
+        if name_match:
+            admonition_name = name_match.group(1)
+            continue
+
         rst_match = re.match(r'^[-=]{3,}$', line.strip())
         if rst_match and previous_line.strip():
             current_heading = previous_line.strip()
@@ -69,14 +100,21 @@ def extract_best_practice_blocks_rst(file_path: pathlib.Path, content: str):
         if inside_admonition:
             at_end = previous_line.strip() == '' and len(line) > 0 and line[0] != ' '
             if at_end:
-                results.append((current_heading, current_ref, '\n'.join(admonition_lines)))
+                results.append((
+                    current_heading,
+                    current_ref,
+                    '\n'.join(admonition_lines),
+                    admonition_name,
+                ))
                 inside_admonition = False
+                admonition_name = None
             else:
                 admonition_lines.append(line)
 
         if re.match(r'^\.\. admonition:: Best practice', line):
             inside_admonition = True
             admonition_lines.clear()
+            admonition_name = None
             previous_line = line
             continue
 
@@ -87,27 +125,34 @@ def extract_best_practice_blocks_rst(file_path: pathlib.Path, content: str):
 
         previous_line = line
 
-    results.sort()
+    results.sort(key=lambda t: (t[0] or '', t[1] or '', t[2]))
     return results
 
 
 def extract_best_practice_blocks_md(file_path: pathlib.Path, content: str):
     """Extracts 'Best practice' blocks from a Markdown file.
 
-    Returns a tuple of (heading, reference, content).
+    Returns a list of (heading, reference, content, name) tuples. ``name`` is
+    the value of the admonition's ``:name:`` option, or ``None`` if not set.
     """
     lines = content.splitlines()
-    results: list[tuple[str | None, str | None, str]] = []
+    results: list[tuple[str | None, str | None, str, str | None]] = []
 
     current_heading = None
     current_ref = None
     inside_admonition = False
     admonition_lines: list[str] = []
+    admonition_name: str | None = None
 
     for line in lines:
         if line.strip() == ':class: hint':
             continue
 
+        name_match = inside_admonition and re.match(r'\s*:name:\s*(\S+)\s*$', line)
+        if name_match:
+            admonition_name = name_match.group(1)
+            continue
+
         md_match = re.match(r'^(#{2,})\s+(.*)', line)
         if md_match:
             current_heading = md_match.group(2)
@@ -116,22 +161,29 @@ def extract_best_practice_blocks_md(file_path: pathlib.Path, content: str):
         if inside_admonition:
             at_end = line.strip() == '```'
             if at_end:
-                results.append((current_heading, current_ref, '\n'.join(admonition_lines)))
+                results.append((
+                    current_heading,
+                    current_ref,
+                    '\n'.join(admonition_lines),
+                    admonition_name,
+                ))
                 inside_admonition = False
+                admonition_name = None
             else:
                 admonition_lines.append(line)
 
         if re.match(r'^```{admonition} Best practice', line):
             inside_admonition = True
             admonition_lines.clear()
+            admonition_name = None
             continue
 
         ref_match = re.match(r'\((.+?)\)=', line)
         if ref_match:
             current_ref = ref_match.group(1)
             continue
 
-    results.sort()
+    results.sort(key=lambda t: (t[0] or '', t[1] or '', t[2]))
     return results
 
 
@@ -158,6 +210,7 @@ def main():
         help='Path to a clone of canonical/charmcraft',
     )
     args = parser.parse_args()
+    print(HEADER)
     path_to_ops = pathlib.Path(__file__).parent.parent
     for directory, base_url, make_ref, ref_sub in (
         (path_to_ops / 'docs', 'https://documentation.ubuntu.com/ops/latest/', make_ops_ref, None),
@@ -188,13 +241,26 @@ def main():
             link = f'{base_url}{file_path.relative_to(directory).with_suffix("")}/'
             if len(practices):
                 print(f'**[{title}]({link})**')
-            for heading, ref, practice in practices:
-                ref = make_ref(heading, ref) if ref and heading else ref
-                see_more = f' See {ref}.' if heading and ref else ''
+            # Practices with no enclosing section link fall back to linking to
+            # their own :name: anchor, but only when the link text (the
+            # heading, or the page title) appears once on this page:
+            # identically-named links with different destinations are
+            # confusing.
+            fallback_texts = [p[0] or title for p in practices if p[3] and not (p[0] and p[1])]
+            for heading, ref, practice, name in practices:
+                if heading and ref:
+                    see_more = f' See {make_ref(heading, ref)}.'
+                elif name and fallback_texts.count(heading or title) == 1:
+                    see_more = f' See {make_ref(heading or title, name)}.'
+                else:
+                    see_more = ''
                 practice = re.sub(r'\s+', ' ', practice).strip()
                 if ref_sub:
                     practice = re.sub(r':ref:', ref_sub, practice)
-                print(f'- {practice}{see_more}')
+                if name:
+                    print(f'- {practice}{see_more} <!-- id: {name} -->')
+                else:
+                    print(f'- {practice}{see_more}')
             if len(practices):
                 print()
 

docs/_static/best-practice-anchors.js

@@ -0,0 +1,12 @@
+document.addEventListener('DOMContentLoaded', () => {
+  document.querySelectorAll('div.admonition[id^="best-practice-"]').forEach((el) => {
+    const title = el.querySelector('.admonition-title');
+    if (!title) return;
+    const a = document.createElement('a');
+    a.className = 'headerlink';
+    a.href = '#' + el.id;
+    a.title = 'Link to this best practice';
+    a.textContent = '¶';
+    title.appendChild(a);
+  });
+});

docs/_static/project_specific.css

@@ -9,6 +9,17 @@ body {
     mask-image: var(--icon-star);
 }
 
+/* Reveal a permalink anchor when hovering a best-practice admonition. The
+   anchor itself is injected by best-practice-anchors.js. */
+div.admonition[id^="best-practice-"] > .admonition-title > a.headerlink {
+    visibility: hidden;
+    margin-left: 0.5em;
+}
+div.admonition[id^="best-practice-"]:hover > .admonition-title > a.headerlink,
+div.admonition[id^="best-practice-"] > .admonition-title > a.headerlink:focus {
+    visibility: visible;
+}
+
 /* Vertically align text in table cells and add an initial horizontal line */
 table.docutils.top-aligned td {
     vertical-align: top;

docs/conf.py

@@ -279,6 +279,7 @@
 # Adds custom JavaScript files, located remotely or in 'html_static_path'.
 html_js_files = [
     "https://assets.ubuntu.com/v1/287a5e8f-bundle.js",
+    "best-practice-anchors.js",
 ]
 
 # Appends extra markup to the end of every document written in reST

docs/howto/initialise-your-project.md

@@ -31,6 +31,7 @@ Create a repository with your source control of choice.
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-repository-naming
 
 Name the repository using the pattern ``<charm name>-operator`` for a single
 charm, or ``<base charm name>-operators`` when the repository will hold

docs/howto/log-from-your-charm.md

@@ -47,6 +47,7 @@ juju debug-log --debug --include-module juju.worker.uniter.operation --include-m
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-use-logging-not-print
 
 Capture output to `stdout` and `stderr` in your charm and use the logging and
 warning functionality to send messages to the charm user, rather than rely on
@@ -72,12 +73,14 @@ logger.info(f"Got some more information {more_info}")
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-meaningful-log-messages
 
 Ensure that log messages are clear, meaningful, and provide enough information for the user to troubleshoot any issues. Avoid spurious logging. For instance, try not to log when event handlers are called, as the Juju controller does this automatically.
 ```
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-no-sensitive-data-in-logs
 
 Never log credentials or other sensitive information.
 ```

docs/howto/manage-charms.md

@@ -44,6 +44,7 @@ The Charmcraft profile has configured some commands to help you develop your cha
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-charmcraft-profile-commands
 
 All charms should provide the commands configured by the Charmcraft profile, to
 allow easy testing across the charm ecosystem. It's fine to tweak the

docs/howto/manage-configuration.md

@@ -12,6 +12,7 @@ In the `charmcraft.yaml` file of the charm, under `config.options`, add a config
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-no-duplicate-model-config
 
 Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config <command-juju-model-config>`.
 ```

docs/howto/manage-libraries.md

@@ -108,6 +108,7 @@ class DatabaseRequirer(ops.Object):
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-libraries-no-status-mutation
 
 Libraries should never mutate the status of a unit or application. Instead, use
 return values, or raise exceptions and let them bubble back up to the charm for

docs/howto/run-workloads-with-a-charm-machines.md

@@ -103,6 +103,7 @@ def uninstall() -> None:
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-pin-workload-versions
 
 Pin workload versions rather than installing the latest available package. A charm that silently upgrades between reconciliations is hard to debug, and can break if upstream introduces a breaking change.
 ```
@@ -144,6 +145,7 @@ If no library is available for installing the workload, use `subprocess` to run
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-safe-subprocess
 
 When running subprocesses, use absolute paths (for example, `/usr/bin/apt` rather than `apt`) to avoid PATH-based attacks, and pass arguments as a list rather than a shell string so that the shell doesn't interpret them. Use `check=True` so that failures raise — generally you'll want to catch that exception and log the return code and possibly `stderr` — and `capture_output=True` so that output from the command doesn't leak into the Juju log.
 ```

docs/howto/set-up-continuous-integration-for-a-charm.md

@@ -9,6 +9,7 @@ myst:
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-automated-ci
 
 The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system.
 ```

docs/howto/write-and-structure-charm-code.md

@@ -25,6 +25,7 @@ charm code that will run with the Python version of the oldest base you support.
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-requires-python
 
 Set the [`requires-python`](https://packaging.python.org/en/latest/specifications/pyproject-toml/#requires-python)
 version in your `pyproject.toml` so that tooling will detect any use of Python
@@ -79,20 +80,23 @@ We recommend that you use `uv add` and `uv remove` instead of editing dependenci
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-automated-dependency-updates
 
 Ensure that tooling is configured to automatically detect new versions,
 particularly security releases, for all your dependencies.
 ```
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-commit-lock-file
 
 Ensure that the `pyproject.toml` *and* the lock file are committed to version
 control, so that exact versions of charms can be reproduced.
 ```
 
 ```{admonition} Best practice
 :class: hint
+:name: best-practice-avoid-charm-plugin
 
 Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin <howto-migrate-to-uv>` or the {external+charmcraft:ref}`poetry plugin <howto-migrate-to-poetry>`.
 ```