From 85b1e51e9b8dd7bc89b984ea17cf8d046ef05e8b Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 2 Jun 2026 21:46:15 +1200 Subject: [PATCH 1/7] docs: give each best-practice admonition a stable :name: anchor Add a :name: option to every Best practice admonition in the operator howto docs, so each practice has its own anchor in the rendered HTML page (e.g. manage-libraries.html#best-practice-libraries-no-status-mutation) and a stable identifier independent of its surrounding text. Update .github/update-best-practice-table.py to extract :name: from each admonition and emit two things in the generated docs/reuse/best-practices.txt: - a Sphinx anchor `()=` before each bullet, so the bullets on the collation page (follow-best-practices section) are also individually deep-linkable - a trailing `` HTML comment, so downstream automation (e.g. canonical/charmhub-listing-review) can match practices by ID rather than fuzzy-matching the description text A header comment at the top of best-practices.txt documents the convention. This is the operator-side of the change; a matching PR against charmcraft will add :name: to the Best practice admonitions there. Admonitions without :name: continue to render exactly as before. Co-Authored-By: Claude Opus 4.7 --- .github/update-best-practice-table.py | 75 ++++++++++++++++--- docs/howto/initialise-your-project.md | 1 + docs/howto/log-from-your-charm.md | 3 + docs/howto/manage-charms.md | 1 + docs/howto/manage-configuration.md | 1 + docs/howto/manage-libraries.md | 1 + .../run-workloads-with-a-charm-machines.md | 2 + ...t-up-continuous-integration-for-a-charm.md | 1 + docs/howto/write-and-structure-charm-code.md | 4 + docs/reuse/best-practices.txt | 54 +++++++++---- 10 files changed, 119 insertions(+), 24 deletions(-) diff --git a/.github/update-best-practice-table.py b/.github/update-best-practice-table.py index 0bc76eb3f..297967546 100755 --- a/.github/update-best-practice-table.py +++ b/.github/update-best-practice-table.py @@ -22,6 +22,12 @@ 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 emits each item as +a Sphinx anchor (``()=``) followed by the bullet line, so consumers can +deep-link to a specific practice, and downstream automation can match practices +by ID via a trailing ```` HTML comment. + ## Local Usage You'll need a clone of the `operator` repository (presumably the one you're running this command @@ -41,25 +47,48 @@ import pathlib import re +# Block at the top of the generated file, explaining how downstream automation +# can locate per-practice identifiers. +HEADER = """\ +`` 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 +98,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 +123,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,14 +159,21 @@ 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) @@ -131,7 +181,7 @@ def extract_best_practice_blocks_md(file_path: pathlib.Path, content: str): 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 +208,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 +239,17 @@ 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: + for heading, ref, practice, name in practices: ref = make_ref(heading, ref) if ref and heading else ref see_more = f' See {ref}.' if heading and ref else '' 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'({name})=') + print(f'- {practice}{see_more} ') + else: + print(f'- {practice}{see_more}') if len(practices): print() diff --git a/docs/howto/initialise-your-project.md b/docs/howto/initialise-your-project.md index e4c54e7b6..284d5bbd2 100644 --- a/docs/howto/initialise-your-project.md +++ b/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 ``-operator`` for a single charm, or ``-operators`` when the repository will hold diff --git a/docs/howto/log-from-your-charm.md b/docs/howto/log-from-your-charm.md index f8038b278..9a5c2daa3 100644 --- a/docs/howto/log-from-your-charm.md +++ b/docs/howto/log-from-your-charm.md @@ -45,6 +45,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 @@ -70,12 +71,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. ``` diff --git a/docs/howto/manage-charms.md b/docs/howto/manage-charms.md index 6729baa45..1924a0fbf 100644 --- a/docs/howto/manage-charms.md +++ b/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 diff --git a/docs/howto/manage-configuration.md b/docs/howto/manage-configuration.md index 46925974a..07d187484 100644 --- a/docs/howto/manage-configuration.md +++ b/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 `. ``` diff --git a/docs/howto/manage-libraries.md b/docs/howto/manage-libraries.md index 4f150003f..3a146fb83 100644 --- a/docs/howto/manage-libraries.md +++ b/docs/howto/manage-libraries.md @@ -97,6 +97,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 diff --git a/docs/howto/run-workloads-with-a-charm-machines.md b/docs/howto/run-workloads-with-a-charm-machines.md index b3592f041..ba619620c 100644 --- a/docs/howto/run-workloads-with-a-charm-machines.md +++ b/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. ``` diff --git a/docs/howto/set-up-continuous-integration-for-a-charm.md b/docs/howto/set-up-continuous-integration-for-a-charm.md index 1bc127c55..d77f5e251 100644 --- a/docs/howto/set-up-continuous-integration-for-a-charm.md +++ b/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. ``` diff --git a/docs/howto/write-and-structure-charm-code.md b/docs/howto/write-and-structure-charm-code.md index e0e141170..17bbc2c8d 100644 --- a/docs/howto/write-and-structure-charm-code.md +++ b/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,6 +80,7 @@ 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. @@ -86,6 +88,7 @@ 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. @@ -93,6 +96,7 @@ 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 ` or the {external+charmcraft:ref}`poetry plugin `. ``` diff --git a/docs/reuse/best-practices.txt b/docs/reuse/best-practices.txt index fa397b01d..f1b194e08 100644 --- a/docs/reuse/best-practices.txt +++ b/docs/reuse/best-practices.txt @@ -1,32 +1,58 @@ +`` 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. +--> + **[How to initialise your project](https://documentation.ubuntu.com/ops/latest/howto/initialise-your-project/)** -- Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold multiple related charms. For the charm name, see [](#decide-your-charms-name). See [Create a repository and initialise it](#create-a-repository-and-initialise-it). +(best-practice-repository-naming)= +- Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold multiple related charms. For the charm name, see [](#decide-your-charms-name). See [Create a repository and initialise it](#create-a-repository-and-initialise-it). **[How to log from your charm](https://documentation.ubuntu.com/ops/latest/howto/log-from-your-charm/)** -- 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 Juju capturing output. In particular, you should avoid `print()` calls, and ensure that any subprocess calls capture output. -- 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. -- Never log credentials or other sensitive information. +(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 Juju capturing output. In particular, you should avoid `print()` calls, and ensure that any subprocess calls capture output. +(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. +(best-practice-no-sensitive-data-in-logs)= +- Never log credentials or other sensitive information. **[How to manage charms](https://documentation.ubuntu.com/ops/latest/howto/manage-charms/)** -- 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 configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides. See [Develop your charm](#develop-your-charm). +(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 configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides. See [Develop your charm](#develop-your-charm). **[How to manage configuration](https://documentation.ubuntu.com/ops/latest/howto/manage-configuration/)** -- Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. See [Define a configuration option](#define-a-configuration-option). +(best-practice-no-duplicate-model-config)= +- Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. See [Define a configuration option](#define-a-configuration-option). **[How to manage libraries](https://documentation.ubuntu.com/ops/latest/howto/manage-libraries/)** -- 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 the charm author to handle as they see fit. See [Write a library](#manage-libraries-write-a-library). +(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 the charm author to handle as they see fit. See [Write a library](#manage-libraries-write-a-library). **[How to run workloads with a machine charm](https://documentation.ubuntu.com/ops/latest/howto/run-workloads-with-a-charm-machines/)** -- 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. See [APT packages](#run-workloads-with-a-charm-machines-apt-packages). -- 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. See [When there's no library](#run-workloads-with-a-charm-machines-when-theres-no-library). +(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. See [APT packages](#run-workloads-with-a-charm-machines-apt-packages). +(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. See [When there's no library](#run-workloads-with-a-charm-machines-when-theres-no-library). **[How to set up continuous integration for a charm](https://documentation.ubuntu.com/ops/latest/howto/set-up-continuous-integration-for-a-charm/)** -- The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. +(best-practice-automated-ci)= +- The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. **[How to write and structure charm code](https://documentation.ubuntu.com/ops/latest/howto/write-and-structure-charm-code/)** -- Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -- Ensure that the `pyproject.toml` *and* the lock file are committed to version control, so that exact versions of charms can be reproduced. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -- Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -- 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 features not available in the versions you support. See [Use the Python provided by the base](#define-the-required-dependencies). +(best-practice-avoid-charm-plugin)= +- Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). +(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. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). +(best-practice-automated-dependency-updates)= +- Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). +(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 features not available in the versions you support. See [Use the Python provided by the base](#define-the-required-dependencies). **[Manage charms](https://documentation.ubuntu.com/charmcraft/latest/howto/manage-charms/)** - If you're setting up a ``git`` repository: name it using the pattern ``-operator``. For the charm name, see {external+charmcraft:ref}`specify-a-name`. See {external+charmcraft:ref}`Initialise a charm `. From c9f3e3adca782288ce917410cba17fac07f6b388 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 2 Jun 2026 22:29:56 +1200 Subject: [PATCH 2/7] docs: skip best-practice anchor on the include host page The generated best-practices.txt emits a Sphinx anchor ``()=`` before each bullet so the collated list is individually deep-linkable. When the admonition lives on the same page that ``{include}``s the collation, that anchor collides with the admonition's own ``:name:`` target and Sphinx fails the build with a duplicate-target warning under --fail-on-warning. Detect include hosts at generation time and skip ``()=`` for their admonitions; the admonition's ``:name:`` already provides the anchor on that page, and the trailing ```` comment is unchanged so downstream automation is unaffected. Co-Authored-By: Claude Opus 4.7 --- .github/update-best-practice-table.py | 29 +++++++++++++++++++++++++-- docs/reuse/best-practices.txt | 9 ++++----- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/.github/update-best-practice-table.py b/.github/update-best-practice-table.py index 297967546..8f2d10dd8 100755 --- a/.github/update-best-practice-table.py +++ b/.github/update-best-practice-table.py @@ -57,12 +57,31 @@ Each practice that declares ``:name: `` in its source admonition is emitted as a Sphinx anchor ``()=`` followed by the bullet line, and is tagged with a trailing HTML comment ```` so that downstream -automation (e.g. charmhub-listing-review) can match practices by ID. +automation (e.g. charmhub-listing-review) can match practices by ID. The +``()=`` anchor is omitted when the admonition lives on the same page as +the ``{include}`` of this file, because the admonition's own ``:name:`` already +provides that anchor on the host page. Do not edit this file directly; update the source admonition instead. --> """ +# Regex used to find pages that ``{include}`` this generated file. Such pages +# already render the source admonitions inline, so emitting ``()=`` for +# their bullets would create duplicate Sphinx targets. +INCLUDE_HOST_RE = re.compile(r'\{include\}[^\n]*best-practices\.txt') + + +def find_include_hosts(docs_dir: pathlib.Path) -> set[pathlib.Path]: + """Return doc files that ``{include}`` the generated best-practices file.""" + hosts: set[pathlib.Path] = set() + for path in docs_dir.rglob('*'): + if path.suffix not in ('.md', '.rst'): + continue + if INCLUDE_HOST_RE.search(path.read_text()): + hosts.add(path) + return hosts + def extract_best_practice_blocks_rst(file_path: pathlib.Path, content: str): """Extracts 'Best practice' blocks from a ReST file. @@ -210,6 +229,7 @@ def main(): args = parser.parse_args() print(HEADER) path_to_ops = pathlib.Path(__file__).parent.parent + include_hosts = find_include_hosts(path_to_ops / 'docs') for directory, base_url, make_ref, ref_sub in ( (path_to_ops / 'docs', 'https://documentation.ubuntu.com/ops/latest/', make_ops_ref, None), ( @@ -237,6 +257,7 @@ def main(): title = mo.group(1).strip() practices = extract_best_practice_blocks_rst(file_path, text) link = f'{base_url}{file_path.relative_to(directory).with_suffix("")}/' + is_include_host = file_path in include_hosts if len(practices): print(f'**[{title}]({link})**') for heading, ref, practice, name in practices: @@ -246,7 +267,11 @@ def main(): if ref_sub: practice = re.sub(r':ref:', ref_sub, practice) if name: - print(f'({name})=') + # Skip the anchor on pages that include this file directly, + # since the source admonition's :name: already provides it + # on the same page (a duplicate target would warn). + if not is_include_host: + print(f'({name})=') print(f'- {practice}{see_more} ') else: print(f'- {practice}{see_more}') diff --git a/docs/reuse/best-practices.txt b/docs/reuse/best-practices.txt index f1b194e08..9b261e956 100644 --- a/docs/reuse/best-practices.txt +++ b/docs/reuse/best-practices.txt @@ -5,7 +5,10 @@ This file is auto-generated by .github/update-best-practice-table.py from Each practice that declares ``:name: `` in its source admonition is emitted as a Sphinx anchor ``()=`` followed by the bullet line, and is tagged with a trailing HTML comment ```` so that downstream -automation (e.g. charmhub-listing-review) can match practices by ID. +automation (e.g. charmhub-listing-review) can match practices by ID. The +``()=`` anchor is omitted when the admonition lives on the same page as +the ``{include}`` of this file, because the admonition's own ``:name:`` already +provides that anchor on the host page. Do not edit this file directly; update the source admonition instead. --> @@ -45,13 +48,9 @@ Do not edit this file directly; update the source admonition instead. - The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. **[How to write and structure charm code](https://documentation.ubuntu.com/ops/latest/howto/write-and-structure-charm-code/)** -(best-practice-avoid-charm-plugin)= - Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -(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. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -(best-practice-automated-dependency-updates)= - Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -(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 features not available in the versions you support. See [Use the Python provided by the base](#define-the-required-dependencies). **[Manage charms](https://documentation.ubuntu.com/charmcraft/latest/howto/manage-charms/)** From 4d56cb32a3058908617860ce18915c6bf281aeae Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 2 Jun 2026 22:38:03 +1200 Subject: [PATCH 3/7] docs: prevent best-practice header from leaking into the rendered page The header at the top of the generated best-practices.txt is an HTML comment, but it contained an example of the trailing per-bullet id comment that included a literal ``-->`` sequence. HTML comments terminate at the first ``-->``, so the outer comment closed early and the remainder of the header rendered as visible text in the ``Follow best practices`` section of the host page. Reword the header so it describes the syntax without including a literal closing sequence, and add a Python-level note explaining why. Co-Authored-By: Claude Opus 4.7 --- .github/update-best-practice-table.py | 23 +++++++++++++---------- docs/reuse/best-practices.txt | 16 ++++++++-------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/.github/update-best-practice-table.py b/.github/update-best-practice-table.py index 8f2d10dd8..71fd899df 100755 --- a/.github/update-best-practice-table.py +++ b/.github/update-best-practice-table.py @@ -48,19 +48,22 @@ import re # Block at the top of the generated file, explaining how downstream automation -# can locate per-practice identifiers. +# 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 = """\ `` so that downstream -automation (e.g. charmhub-listing-review) can match practices by ID. The -``()=`` anchor is omitted when the admonition lives on the same page as -the ``{include}`` of this file, because the admonition's own ``:name:`` already -provides that anchor on the host page. +"Best practice" admonitions in the operator and charmcraft documentation. + +Each practice that declares a :name: slug in its source admonition is +emitted as a Sphinx anchor (slug)= on the line before the bullet, and the +bullet is tagged with a trailing HTML id-comment containing the slug so +that downstream automation (e.g. charmhub-listing-review) can match +practices by ID. The (slug)= anchor is omitted when the admonition lives +on the same page as the include of this file, because the admonition's +own :name: already provides that anchor on the host page. Do not edit this file directly; update the source admonition instead. --> diff --git a/docs/reuse/best-practices.txt b/docs/reuse/best-practices.txt index 9b261e956..373c22c3a 100644 --- a/docs/reuse/best-practices.txt +++ b/docs/reuse/best-practices.txt @@ -1,14 +1,14 @@ `` so that downstream -automation (e.g. charmhub-listing-review) can match practices by ID. The -``()=`` anchor is omitted when the admonition lives on the same page as -the ``{include}`` of this file, because the admonition's own ``:name:`` already -provides that anchor on the host page. +Each practice that declares a :name: slug in its source admonition is +emitted as a Sphinx anchor (slug)= on the line before the bullet, and the +bullet is tagged with a trailing HTML id-comment containing the slug so +that downstream automation (e.g. charmhub-listing-review) can match +practices by ID. The (slug)= anchor is omitted when the admonition lives +on the same page as the include of this file, because the admonition's +own :name: already provides that anchor on the host page. Do not edit this file directly; update the source admonition instead. --> From 5b9e876255c14d34d44892fd5944c5e686ce71fb Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 5 Jun 2026 21:13:10 +1200 Subject: [PATCH 4/7] docs: add hover permalink anchor to best-practice admonitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each best-practice admonition already gets a stable id from its :name:, but the anchor is invisible in the rendered page. Inject a ¶ headerlink into the admonition title (revealed on hover) so readers can copy a deep link the same way they would from a heading. Co-Authored-By: Claude Opus 4.7 --- docs/_static/best-practice-anchors.js | 12 ++++++++++++ docs/_static/project_specific.css | 11 +++++++++++ docs/conf.py | 1 + 3 files changed, 24 insertions(+) create mode 100644 docs/_static/best-practice-anchors.js diff --git a/docs/_static/best-practice-anchors.js b/docs/_static/best-practice-anchors.js new file mode 100644 index 000000000..5a0f67cb5 --- /dev/null +++ b/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); + }); +}); diff --git a/docs/_static/project_specific.css b/docs/_static/project_specific.css index 9f5b2d133..cea75f7f4 100644 --- a/docs/_static/project_specific.css +++ b/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; diff --git a/docs/conf.py b/docs/conf.py index f9b497fc7..2d5e7f2dd 100644 --- a/docs/conf.py +++ b/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 From 6fc24ccf2377db2d1d2c07b7e2ee5f0b2e34e240 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 10 Jun 2026 23:39:16 +1200 Subject: [PATCH 5/7] docs: drop the (slug)= anchors from the collated best practices Per review, deep links to the bullets on the collation page aren't needed: the JS hover anchors on the source admonitions cover linking to individual practices, and the trailing comments cover downstream automation. Stop emitting the (slug)= targets, and remove the include-host detection that only existed to avoid duplicating them. Co-Authored-By: Claude Fable 5 --- .github/update-best-practice-table.py | 41 +++++---------------------- docs/reuse/best-practices.txt | 21 +++----------- 2 files changed, 11 insertions(+), 51 deletions(-) diff --git a/.github/update-best-practice-table.py b/.github/update-best-practice-table.py index 71fd899df..f5f43b3c0 100755 --- a/.github/update-best-practice-table.py +++ b/.github/update-best-practice-table.py @@ -23,10 +23,9 @@ Ops documentation. Each ``Best practice`` admonition may declare a ``:name:`` option, which is used -as a stable identifier for the practice. The generated output emits each item as -a Sphinx anchor (``()=``) followed by the bullet line, so consumers can -deep-link to a specific practice, and downstream automation can match practices -by ID via a trailing ```` HTML comment. +as a stable identifier for the practice. The generated output tags each bullet +with a trailing ```` HTML comment, so downstream automation +can match practices by ID. ## Local Usage @@ -57,34 +56,15 @@ 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 is -emitted as a Sphinx anchor (slug)= on the line before the bullet, and the -bullet is tagged with a trailing HTML id-comment containing the slug so -that downstream automation (e.g. charmhub-listing-review) can match -practices by ID. The (slug)= anchor is omitted when the admonition lives -on the same page as the include of this file, because the admonition's -own :name: already provides that anchor on the host page. +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. --> """ -# Regex used to find pages that ``{include}`` this generated file. Such pages -# already render the source admonitions inline, so emitting ``()=`` for -# their bullets would create duplicate Sphinx targets. -INCLUDE_HOST_RE = re.compile(r'\{include\}[^\n]*best-practices\.txt') - - -def find_include_hosts(docs_dir: pathlib.Path) -> set[pathlib.Path]: - """Return doc files that ``{include}`` the generated best-practices file.""" - hosts: set[pathlib.Path] = set() - for path in docs_dir.rglob('*'): - if path.suffix not in ('.md', '.rst'): - continue - if INCLUDE_HOST_RE.search(path.read_text()): - hosts.add(path) - return hosts - def extract_best_practice_blocks_rst(file_path: pathlib.Path, content: str): """Extracts 'Best practice' blocks from a ReST file. @@ -232,7 +212,6 @@ def main(): args = parser.parse_args() print(HEADER) path_to_ops = pathlib.Path(__file__).parent.parent - include_hosts = find_include_hosts(path_to_ops / 'docs') for directory, base_url, make_ref, ref_sub in ( (path_to_ops / 'docs', 'https://documentation.ubuntu.com/ops/latest/', make_ops_ref, None), ( @@ -260,7 +239,6 @@ def main(): title = mo.group(1).strip() practices = extract_best_practice_blocks_rst(file_path, text) link = f'{base_url}{file_path.relative_to(directory).with_suffix("")}/' - is_include_host = file_path in include_hosts if len(practices): print(f'**[{title}]({link})**') for heading, ref, practice, name in practices: @@ -270,11 +248,6 @@ def main(): if ref_sub: practice = re.sub(r':ref:', ref_sub, practice) if name: - # Skip the anchor on pages that include this file directly, - # since the source admonition's :name: already provides it - # on the same page (a duplicate target would warn). - if not is_include_host: - print(f'({name})=') print(f'- {practice}{see_more} ') else: print(f'- {practice}{see_more}') diff --git a/docs/reuse/best-practices.txt b/docs/reuse/best-practices.txt index 373c22c3a..0ddfc4c7f 100644 --- a/docs/reuse/best-practices.txt +++ b/docs/reuse/best-practices.txt @@ -2,49 +2,36 @@ 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 is -emitted as a Sphinx anchor (slug)= on the line before the bullet, and the -bullet is tagged with a trailing HTML id-comment containing the slug so -that downstream automation (e.g. charmhub-listing-review) can match -practices by ID. The (slug)= anchor is omitted when the admonition lives -on the same page as the include of this file, because the admonition's -own :name: already provides that anchor on the host page. +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. --> **[How to initialise your project](https://documentation.ubuntu.com/ops/latest/howto/initialise-your-project/)** -(best-practice-repository-naming)= - Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold multiple related charms. For the charm name, see [](#decide-your-charms-name). See [Create a repository and initialise it](#create-a-repository-and-initialise-it). **[How to log from your charm](https://documentation.ubuntu.com/ops/latest/howto/log-from-your-charm/)** -(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 Juju capturing output. In particular, you should avoid `print()` calls, and ensure that any subprocess calls capture output. -(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. -(best-practice-no-sensitive-data-in-logs)= - Never log credentials or other sensitive information. **[How to manage charms](https://documentation.ubuntu.com/ops/latest/howto/manage-charms/)** -(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 configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides. See [Develop your charm](#develop-your-charm). **[How to manage configuration](https://documentation.ubuntu.com/ops/latest/howto/manage-configuration/)** -(best-practice-no-duplicate-model-config)= - Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. See [Define a configuration option](#define-a-configuration-option). **[How to manage libraries](https://documentation.ubuntu.com/ops/latest/howto/manage-libraries/)** -(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 the charm author to handle as they see fit. See [Write a library](#manage-libraries-write-a-library). **[How to run workloads with a machine charm](https://documentation.ubuntu.com/ops/latest/howto/run-workloads-with-a-charm-machines/)** -(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. See [APT packages](#run-workloads-with-a-charm-machines-apt-packages). -(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. See [When there's no library](#run-workloads-with-a-charm-machines-when-theres-no-library). **[How to set up continuous integration for a charm](https://documentation.ubuntu.com/ops/latest/howto/set-up-continuous-integration-for-a-charm/)** -(best-practice-automated-ci)= - The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. **[How to write and structure charm code](https://documentation.ubuntu.com/ops/latest/howto/write-and-structure-charm-code/)** From 8d3c63536c92cfa23253d84d83f94a4c3ca37467 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 11 Jun 2026 11:20:09 +1200 Subject: [PATCH 6/7] docs: link collated best practices to their :name: anchors Per review, point the trailing "See" link on each collated bullet at the practice's own :name: anchor, so readers land on the exact admonition rather than the enclosing section. Also give bullets that previously had no "See" link (practices that aren't under a section heading) a link, using the page title as the link text. The charmcraft-sourced bullets are unchanged; those admonitions don't have :name: options yet and will be handled in a separate charmcraft PR. Co-Authored-By: Claude Fable 5 --- .github/update-best-practice-table.py | 12 ++++++++++-- docs/reuse/best-practices.txt | 28 +++++++++++++-------------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/.github/update-best-practice-table.py b/.github/update-best-practice-table.py index f5f43b3c0..72d77c853 100755 --- a/.github/update-best-practice-table.py +++ b/.github/update-best-practice-table.py @@ -242,8 +242,16 @@ def main(): if len(practices): print(f'**[{title}]({link})**') for heading, ref, practice, name in practices: - ref = make_ref(heading, ref) if ref and heading else ref - see_more = f' See {ref}.' if heading and ref else '' + # Link directly to the practice's own :name: anchor when it has + # one, rather than to the enclosing section. Practices that + # aren't under a section heading use the page title as the + # link text. + if name: + see_more = f' See {make_ref(heading or title, name)}.' + elif heading and ref: + see_more = f' See {make_ref(heading, ref)}.' + else: + see_more = '' practice = re.sub(r'\s+', ' ', practice).strip() if ref_sub: practice = re.sub(r':ref:', ref_sub, practice) diff --git a/docs/reuse/best-practices.txt b/docs/reuse/best-practices.txt index 0ddfc4c7f..92a2ce415 100644 --- a/docs/reuse/best-practices.txt +++ b/docs/reuse/best-practices.txt @@ -11,34 +11,34 @@ Do not edit this file directly; update the source admonition instead. --> **[How to initialise your project](https://documentation.ubuntu.com/ops/latest/howto/initialise-your-project/)** -- Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold multiple related charms. For the charm name, see [](#decide-your-charms-name). See [Create a repository and initialise it](#create-a-repository-and-initialise-it). +- Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold multiple related charms. For the charm name, see [](#decide-your-charms-name). See [Create a repository and initialise it](#best-practice-repository-naming). **[How to log from your charm](https://documentation.ubuntu.com/ops/latest/howto/log-from-your-charm/)** -- 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 Juju capturing output. In particular, you should avoid `print()` calls, and ensure that any subprocess calls capture output. -- 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. -- Never log credentials or other sensitive information. +- 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 Juju capturing output. In particular, you should avoid `print()` calls, and ensure that any subprocess calls capture output. See [How to log from your charm](#best-practice-use-logging-not-print). +- 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. See [How to log from your charm](#best-practice-meaningful-log-messages). +- Never log credentials or other sensitive information. See [How to log from your charm](#best-practice-no-sensitive-data-in-logs). **[How to manage charms](https://documentation.ubuntu.com/ops/latest/howto/manage-charms/)** -- 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 configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides. See [Develop your charm](#develop-your-charm). +- 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 configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides. See [Develop your charm](#best-practice-charmcraft-profile-commands). **[How to manage configuration](https://documentation.ubuntu.com/ops/latest/howto/manage-configuration/)** -- Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. See [Define a configuration option](#define-a-configuration-option). +- Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. See [Define a configuration option](#best-practice-no-duplicate-model-config). **[How to manage libraries](https://documentation.ubuntu.com/ops/latest/howto/manage-libraries/)** -- 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 the charm author to handle as they see fit. See [Write a library](#manage-libraries-write-a-library). +- 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 the charm author to handle as they see fit. See [Write a library](#best-practice-libraries-no-status-mutation). **[How to run workloads with a machine charm](https://documentation.ubuntu.com/ops/latest/howto/run-workloads-with-a-charm-machines/)** -- 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. See [APT packages](#run-workloads-with-a-charm-machines-apt-packages). -- 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. See [When there's no library](#run-workloads-with-a-charm-machines-when-theres-no-library). +- 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. See [APT packages](#best-practice-pin-workload-versions). +- 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. See [When there's no library](#best-practice-safe-subprocess). **[How to set up continuous integration for a charm](https://documentation.ubuntu.com/ops/latest/howto/set-up-continuous-integration-for-a-charm/)** -- The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. +- The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. See [How to set up continuous integration for a charm](#best-practice-automated-ci). **[How to write and structure charm code](https://documentation.ubuntu.com/ops/latest/howto/write-and-structure-charm-code/)** -- Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -- Ensure that the `pyproject.toml` *and* the lock file are committed to version control, so that exact versions of charms can be reproduced. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -- Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -- 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 features not available in the versions you support. See [Use the Python provided by the base](#define-the-required-dependencies). +- Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. See [Add Python dependencies to pyproject.toml and update the lock file](#best-practice-avoid-charm-plugin). +- Ensure that the `pyproject.toml` *and* the lock file are committed to version control, so that exact versions of charms can be reproduced. See [Add Python dependencies to pyproject.toml and update the lock file](#best-practice-commit-lock-file). +- Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. See [Add Python dependencies to pyproject.toml and update the lock file](#best-practice-automated-dependency-updates). +- 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 features not available in the versions you support. See [Use the Python provided by the base](#best-practice-requires-python). **[Manage charms](https://documentation.ubuntu.com/charmcraft/latest/howto/manage-charms/)** - If you're setting up a ``git`` repository: name it using the pattern ``-operator``. For the charm name, see {external+charmcraft:ref}`specify-a-name`. See {external+charmcraft:ref}`Initialise a charm `. From 43d228ad2315e8aa10b2aec70ded4b373eda9ea0 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 11 Jun 2026 18:31:59 +1200 Subject: [PATCH 7/7] docs: revert collated best practice links to section anchors Landing on the enclosing section gives the reader more context than landing on the admonition itself, and retargeting also produced multiple links with the same text but different destinations (e.g. the three logging practices all linked as "How to log from your charm"). Keep the :name:-anchor fallback only for practices that previously had no "See" link at all, and only when the link text appears once on the page, so identically-named links can't point at different destinations. Co-Authored-By: Claude Fable 5 --- .github/update-best-practice-table.py | 16 +++++++++------- docs/reuse/best-practices.txt | 26 +++++++++++++------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/.github/update-best-practice-table.py b/.github/update-best-practice-table.py index 72d77c853..17847ac3d 100755 --- a/.github/update-best-practice-table.py +++ b/.github/update-best-practice-table.py @@ -241,15 +241,17 @@ def main(): link = f'{base_url}{file_path.relative_to(directory).with_suffix("")}/' if len(practices): print(f'**[{title}]({link})**') + # 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: - # Link directly to the practice's own :name: anchor when it has - # one, rather than to the enclosing section. Practices that - # aren't under a section heading use the page title as the - # link text. - if name: - see_more = f' See {make_ref(heading or title, name)}.' - elif heading and ref: + 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() diff --git a/docs/reuse/best-practices.txt b/docs/reuse/best-practices.txt index 92a2ce415..7f7bf7a34 100644 --- a/docs/reuse/best-practices.txt +++ b/docs/reuse/best-practices.txt @@ -11,34 +11,34 @@ Do not edit this file directly; update the source admonition instead. --> **[How to initialise your project](https://documentation.ubuntu.com/ops/latest/howto/initialise-your-project/)** -- Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold multiple related charms. For the charm name, see [](#decide-your-charms-name). See [Create a repository and initialise it](#best-practice-repository-naming). +- Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold multiple related charms. For the charm name, see [](#decide-your-charms-name). See [Create a repository and initialise it](#create-a-repository-and-initialise-it). **[How to log from your charm](https://documentation.ubuntu.com/ops/latest/howto/log-from-your-charm/)** -- 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 Juju capturing output. In particular, you should avoid `print()` calls, and ensure that any subprocess calls capture output. See [How to log from your charm](#best-practice-use-logging-not-print). -- 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. See [How to log from your charm](#best-practice-meaningful-log-messages). -- Never log credentials or other sensitive information. See [How to log from your charm](#best-practice-no-sensitive-data-in-logs). +- 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 Juju capturing output. In particular, you should avoid `print()` calls, and ensure that any subprocess calls capture output. +- 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. +- Never log credentials or other sensitive information. **[How to manage charms](https://documentation.ubuntu.com/ops/latest/howto/manage-charms/)** -- 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 configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides. See [Develop your charm](#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 configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides. See [Develop your charm](#develop-your-charm). **[How to manage configuration](https://documentation.ubuntu.com/ops/latest/howto/manage-configuration/)** -- Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. See [Define a configuration option](#best-practice-no-duplicate-model-config). +- Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. See [Define a configuration option](#define-a-configuration-option). **[How to manage libraries](https://documentation.ubuntu.com/ops/latest/howto/manage-libraries/)** -- 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 the charm author to handle as they see fit. See [Write a library](#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 the charm author to handle as they see fit. See [Write a library](#manage-libraries-write-a-library). **[How to run workloads with a machine charm](https://documentation.ubuntu.com/ops/latest/howto/run-workloads-with-a-charm-machines/)** -- 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. See [APT packages](#best-practice-pin-workload-versions). -- 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. See [When there's no library](#best-practice-safe-subprocess). +- 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. See [APT packages](#run-workloads-with-a-charm-machines-apt-packages). +- 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. See [When there's no library](#run-workloads-with-a-charm-machines-when-theres-no-library). **[How to set up continuous integration for a charm](https://documentation.ubuntu.com/ops/latest/howto/set-up-continuous-integration-for-a-charm/)** - The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. See [How to set up continuous integration for a charm](#best-practice-automated-ci). **[How to write and structure charm code](https://documentation.ubuntu.com/ops/latest/howto/write-and-structure-charm-code/)** -- Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. See [Add Python dependencies to pyproject.toml and update the lock file](#best-practice-avoid-charm-plugin). -- Ensure that the `pyproject.toml` *and* the lock file are committed to version control, so that exact versions of charms can be reproduced. See [Add Python dependencies to pyproject.toml and update the lock file](#best-practice-commit-lock-file). -- Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. See [Add Python dependencies to pyproject.toml and update the lock file](#best-practice-automated-dependency-updates). -- 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 features not available in the versions you support. See [Use the Python provided by the base](#best-practice-requires-python). +- Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). +- Ensure that the `pyproject.toml` *and* the lock file are committed to version control, so that exact versions of charms can be reproduced. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). +- Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). +- 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 features not available in the versions you support. See [Use the Python provided by the base](#define-the-required-dependencies). **[Manage charms](https://documentation.ubuntu.com/charmcraft/latest/howto/manage-charms/)** - If you're setting up a ``git`` repository: name it using the pattern ``-operator``. For the charm name, see {external+charmcraft:ref}`specify-a-name`. See {external+charmcraft:ref}`Initialise a charm `.