Documentation site at talm.cozystack.io with a generated CLI reference - #235
Conversation
|
Important Review skippedToo many files! This PR contains 116 files, which is 16 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (116)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
myasnikovdaniil
left a comment
There was a problem hiding this comment.
APPROVE. I ran go test ./... and mkdocs build --strict, both pass. I also checked navigation, search and mobile layout in browser. Pages needs to be enabled manually with GitHub Actions as source when setting the custom domain.
talm's only documentation was the README, which never covered the command surface: five native commands plus everything wrapped from talosctl, each with its own flags. Writing that by hand would go stale on the next Talos bump, since the wrapper re-exports whatever upstream ships. Render it instead. TestReferenceDocs walks the live command tree and compares it against the committed pages, so `go test ./...` fails when the two drift; `make docs` rewrites them. This follows the golden-file convention already used for the chart render snapshots. The pages are rendered per command rather than through doc.GenMarkdownTree, which hardcodes file names carrying a redundant talm_ prefix and opens each page at H2, leaving no H1 for a docs site to title it from. Cobra's auto-generated date stamp is also suppressed on every command, not just those with a parent, so repeated runs agree. The test skips on Windows: flag defaults are built with filepath.Join, so the rendered pages carry backslashes there and a byte comparison fails for reasons unrelated to drift. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
The README carried the entire manual: 459 lines covering everything from installing the binary to age-encrypted user values, in one flat page with no navigation, no search and no way to link a reader to a single topic. Sections had grown into each other — the getting-started walk had 46 lines of format reference and a 1000-word block on apply-time gates wedged between "gather node information" and "apply config". Split it across pages at those seams and build the result with MkDocs Material, deployed to talm.cozystack.io by GitHub Pages. The prose is moved, not rewritten; the parts that change are heading levels, the one cross-reference that pointed at a section now on another page, and blockquotes that carried warnings, which become admonitions. The README keeps the pitch, the features and the install instructions, and links out for the rest. mkdocs.yml raises MkDocs' anchor, link and nav validation to warnings so that `--strict` fails on a cross-reference lost in a future split rather than shipping it. Both the deploy and the pull-request check build with that flag; the pull-request check carries no paths filter, since a workflow skipped by one stays Pending forever and could never be a required check. The test pinning the post-upgrade reconcile window against a hardcoded 90s followed its subject out of the README and now reads the page that documents it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
talm warned that strvals reads a dot in a --set value as YAML key
nesting, so that `--set endpoint=10.0.0.1` rendered as
{endpoint: {10: {0: {0: 1}}}}, and steered operators to --set-string for
IP, CIDR and version literals. The flag help and the manual said the
same thing.
It does not. strvals stops key scanning at '=', so a dot only opens a
nesting level on the left of it; the value scanner stops at ',' and
typedVal falls back to the raw string once ParseInt fails. Behaviour is
identical on helm v3.15.2 and v4, so the migration between them did not
introduce this. The screen fired on shapes that were never at risk and
told operators their config was silently corrupt when it was not.
Drop the screen and describe what --set actually does: it converts
integers and booleans, which is the real and only reason to prefer
--set-string. A new engine test pins the parser behaviour the old text
described, and the help contract now rejects the claim instead of
requiring it.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
1a0fd98 to
b012d20
Compare
DNS for
talm.cozystack.ioresolves, so here is a site to put behind it.The README was the whole manual: 459 lines, one page, no navigation, no search, no command reference. Sections had also grown into each other, with a 1000-word block on apply-time gates sitting between "gather node information" and "apply config". This splits it into pages under
docs/, built with MkDocs Material. The prose moves as-is. What changes: heading levels, one cross-reference that pointed at a section now living on another page, and warning blockquotes, which become admonitions. The README keeps the pitch, the features and the install instructions, and links out for the rest.The CLI reference is generated from the cobra command tree.
TestReferenceDocscompares the committed pages against that tree, sogo test ./...fails on drift andmake docsregenerates them. talm re-exports talosctl's commands, so a Talos bump moves the command surface without anyone touching talm code.It also drops a false claim that would otherwise have been carried straight into the new docs. talm warned that
--set endpoint=10.0.0.1reads the dots as key nesting and rendered{endpoint: {10: {0: {0: 1}}}}, and told operators to use--set-stringfor IP, CIDR and version literals. strvals does not do that on helm v3 or v4: dots nest only on the left of the=, and the value comes through whole. So the warning fired on values that were never at risk and told operators their config was corrupt when it was not. The screen is gone, the flag help now names the real difference (integers and booleans get converted), and two tests pin the parser behaviour so the claim cannot come back.Before merging, someone has to set the custom domain to
talm.cozystack.ioin Settings > Pages. There is noCNAMEfile, because GitHub ignores one in the artifact when publishing from a workflow, and the domain is not expressible in the workflow either. Until it is set, the 13 absolute links now in the README 404. Provisioning the Pages site itself needs no manual step, the workflow does that throughconfigure-pages.Worth knowing while reviewing:
mkdocs build --strictruns on every PR with anchor, link and nav validation raised to warnings. That workflow has no paths filter on purpose: one skipped by a filter stays Pending and can never be a required check.go-md2manarrives as an indirect dependency ofcobra/doc. It stays test-only, so the shipped binary is unchanged.filepath.Join, so the pages render with backslashes there.cozystack/websitealso documents talm undercontent/en/docs/*/install/kubernetes/talm.md, so the two overlap now. Nothing there links to a README anchor, so nothing breaks, but the duplication is worth a follow-up in that repo.