Skip to content

docs(ansible): align agnocast README with the role - #7293

Merged
xmfcx merged 4 commits into
mainfrom
docs/agnocast-readme
Sep 1, 2026
Merged

docs(ansible): align agnocast README with the role#7293
xmfcx merged 4 commits into
mainfrom
docs/agnocast-readme

Conversation

@xmfcx

@xmfcx xmfcx commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Description

The agnocast role README described the role at v2.1.2 and carried a shell copy of the tasks. That copy drifted from the role: an old version, add-apt-repository where the role now uses a keyring and a .sources file, and a dkms check that never matched. This PR removes the copy. The Ansible command is the only installation step, as in the acados and qt5ct_setup READMEs.

  • Point the upstream link to autowarefoundation/agnocast.
  • List the three overridable defaults under Inputs. agnocast_force_kmod_install is left out, because it only has an effect inside a container.
  • Add an Installation section with the --tags agnocast command and the -e agnocast_version=<version> override.
  • Remove the Manual Installation section.

The first version of this PR kept the snippet and updated it. The review comment on this PR pointed out that the add-apt-repository line contradicts the role, and that the headers step overstated the role. Both points are moot without the snippet.

This PR also carries one CODEOWNERS line. A PR with that line was based on this branch and merged into it.

How to verify

  1. Make sure that the tag in the ansible-playbook command matches ansible/playbooks/install_dev_env.yaml.
  2. Make sure that the three inputs match ansible/roles/agnocast/defaults/main.yaml.
  3. Make sure that the #ansible-installation anchor exists in ansible/README.md.

AI usage

AI usage: written with Claude Code, from a comparison of the README against tasks/main.yaml and defaults/main.yaml. The rewrite after review was done the same way.

Self-review: I ran the whole thing and it works well. I checked every change and approve it, can merge 👍

Verification: pre-commit passed on the file. The tag, the inputs, and the anchor were checked against the source files. The ansible-playbook command did not run in the authoring session.

Comparison against the role

Read tasks/main.yaml, defaults/main.yaml, and ansible/playbooks/install_dev_env.yaml.

  • The tag agnocast is read from ansible/playbooks/install_dev_env.yaml, line 45
  • The three inputs and their defaults come from ansible/roles/agnocast/defaults/main.yaml
  • The #ansible-installation anchor exists in ansible/README.md, line 7
  • Not run: the ansible-playbook --tags agnocast command. It needs sudo, and the command shape is copied from acados and qt5ct_setup

pre-commit

pre-commit run --files ansible/roles/agnocast/README.md

  • markdownlint passed, prettier passed
  • Not run: pre-commit run --all-files

The README described the role at v2.1.2. The tasks changed after that.

- Set the version to 2.3.5.
- Point the upstream link to autowarefoundation/agnocast.
- List the three overridable defaults under Inputs.
- Add the ansible-playbook command with the agnocast tag.
- Install the kernel headers before the kmod, as the role does.
- Check the dkms state with `dkms status agnocast/<version> | grep -q installed`. The old check had no `$` in `{agnocast_version}` and never matched.
- Load the module at boot via /etc/modules-load.d/agnocast.conf.

Signed-off-by: Mete Fatih Cırıt <mfc@autoware.org>
@xmfcx xmfcx added the type:documentation Creating or refining documentation. label Sep 1, 2026
@xmfcx xmfcx self-assigned this Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@xmfcx xmfcx added the run:health-check Run health-check label Sep 1, 2026
@xmfcx
xmfcx marked this pull request as ready for review September 1, 2026 08:19
@xmfcx
xmfcx requested a review from Koichi98 September 1, 2026 08:19
CODEOWNERS is last-match-wins with no owner merging, so the new line repeats the ansible/** owners and appends @Koichi98.

Signed-off-by: Mete Fatih Cırıt <mfc@autoware.org>
@Koichi98

Koichi98 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

One nit on the description: the role installs linux-headers only when apt-cache search finds it, with ignore_errors: true (tasks/main.yaml:151-159), so "as the role does since #5930" overstates it slightly. Separately, the add-apt-repository line predates this PR but is now stale — the role's first three tasks (tasks/main.yaml:2-25) explicitly remove what that command creates, after the keyring + .sources move in #6603/#6670 — so I'm happy to send a follow-up PR for it unless you'd rather fold it in here.

@xmfcx
xmfcx marked this pull request as draft September 1, 2026 12:42
The snippet was a second copy of the role in shell, and it drifted: an old version, add-apt-repository, and a dkms check that never matched. The Ansible command is now the only installation step, as in the acados and qt5ct_setup READMEs.

Signed-off-by: Mete Fatih Cırıt <mfc@autoware.org>
@xmfcx

xmfcx commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, both points are right.

Rather than keep a shell copy of the role in the README, I dropped the manual snippet. The README now has the Inputs table and the ansible-playbook --tags agnocast command only, like the acados and qt5ct_setup READMEs. That removes the stale add-apt-repository line and the headers step together, so no follow-up PR is needed.

@xmfcx
xmfcx marked this pull request as ready for review September 1, 2026 12:47
@Koichi98

Koichi98 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

spell-check-differential is failing on .github/CODEOWNERS:6 — cspell flags Koichi as an unknown word. Using my email koichi.imai.2@tier4.jp instead of @Koichi98 should fix it, and it also matches the other lines, which are all email-based. Should have caught this in #7294 — worth fixing here.

The other owners in the file are email addresses. The GitHub handle was the only exception.

Signed-off-by: Mete Fatih Cırıt <mfc@autoware.org>
@xmfcx

xmfcx commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, spell-check-differential is not required CI in this repository but updated it with your email, could you review again?

@xmfcx
xmfcx enabled auto-merge (squash) September 1, 2026 13:42
@xmfcx
xmfcx merged commit 2c73915 into main Sep 1, 2026
23 checks passed
@xmfcx
xmfcx deleted the docs/agnocast-readme branch September 1, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run:health-check Run health-check type:documentation Creating or refining documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants