From: Anderson A. Giacomolli Date: Wed, 11 Nov 2026 20:38:35 +0000 (-0300) Subject: Share general review and documentation rules X-Git-Url: https://git.giacomolli.uk/onechat/commitdiff_plain/f4509310b5abe5012cb83bed3960404e8fa92217?hp=9346d20fe4297dce1aa16171749a7ef3e41ce7bd Share general review and documentation rules General error, cancellation, test, and documentation rules keep contracts precise without importing another project's package layout. Multiline boundaries, real-path tests for text-dependent errors, required test dependencies, and support claims have explicit rules. Signature verification can use a temporary allowed-signers file without configuration changes. Discussion-derived changes include an archive link. The existing code placement and signing exception stay intact. Issue: 3cdc678 --- diff --git a/.agents/skills/commit/SKILL.md b/.agents/skills/commit/SKILL.md index 11b5eb2..35c8be8 100644 --- a/.agents/skills/commit/SKILL.md +++ b/.agents/skills/commit/SKILL.md @@ -30,8 +30,10 @@ exception does not permit unsigned code or documentation commits. commit hook still runs. Never skip it with `--no-verify`. 6. Cryptographically verify each new normal commit's signature against the configured key before reporting completion. A signature header - alone is not verification. If verification fails, stop and report it. - Do not claim completion or publish the commit. + alone is not verification. If `gpg.ssh.allowedSignersFile` is not + set, use a temporary allowed-signers file with the configured key. Do + not change the Git configuration. If verification fails, stop and + report it. Do not claim completion or publish the commit. ## Size @@ -75,6 +77,8 @@ exception does not permit unsigned code or documentation commits. `git interpret-trailers --trailer` before the message check. Never use a closing keyword such as "Fixes" or "Closes". - Link the bug report when a change fixes a reported bug. +- Include a stable archive URL when a change comes from a mailing-list + or forum discussion. - Do not add an AI tool as co-author. Do not add attribution trailers. ## Example diff --git a/.agents/skills/docs/SKILL.md b/.agents/skills/docs/SKILL.md index 2f7f3dc..b95c3ba 100644 --- a/.agents/skills/docs/SKILL.md +++ b/.agents/skills/docs/SKILL.md @@ -5,6 +5,9 @@ description: Docs and comment rules. Use before you write any prose. # Documentation and comments +Change documentation and comments in the same change as the behavior, +interface, or constraint that they describe. + ## Documentation - `README.rst` is the entry point. It says what OneChat is, its status, @@ -17,9 +20,34 @@ description: Docs and comment rules. Use before you write any prose. in the Go doc comments of the code that owns them. - Do not add other documentation files or a `docs/` folder. Document commands and flags in the command help, and code contracts and design - reasons in Go doc comments. + reasons in Go doc comments. Keep help and Go comments consistent with + the implementation. - Use Markdown only for files that a tool requires by name, such as `AGENTS.md` and `SKILL.md`. +- Follow + [Google's timeless documentation guide](https://developers.google.com/style/timeless-documentation). + Describe what the checked-in code does. Do not use labels that depend + on time, such as "new", "now", "currently", and "previously". +- Follow + [Google's tense guide](https://developers.google.com/style/tense). Use + the present tense for general behavior. Use the future tense only when + the timing of an action requires it, not for planned features. +- Keep previous designs, development history, roadmaps, and future work + in issues. +- State exact support and limits instead of labels such as "prototype". + Keep implemented, tested, and unverified behavior apart. +- Keep safety requirements and known limits explicit. For example, say + when fake-service tests do not verify compatibility with the live + service. +- Keep compatibility and old-schema details when the code or the + migration tests require them. They describe an active contract, not + history. +- Do not remove a warning or claim support only to remove historical + wording. Check the implementation and the tests before you change a + claim. +- Compare claims, examples, paths, and links with the implementation and + the tests. Make sure that examples use supported interfaces and + contain no secrets. ## Lists @@ -30,9 +58,13 @@ description: Docs and comment rules. Use before you write any prose. ## Comments - A comment explains the declaration that it belongs to: the contract, - invariants, and non-obvious reasons. Do not repeat what the code shows - or describe unrelated caller behavior. -- Update or remove a comment when its code changes. + invariants, non-obvious reasons, and required compatibility behavior. + Do not repeat what the code shows or describe unrelated caller + behavior. +- State input constraints, results, errors, and concurrency or resource + ownership rules when a caller needs them. +- Keep a comment next to the code that it describes. Update or remove a + comment when its code changes. - Write prose comments as continuous paragraphs, with one space between words and sentences. Do not use semicolons in prose. - Reflow each changed comment paragraph with `bin/reflow`. Do not reflow @@ -45,3 +77,9 @@ This example reflows the comment on lines 12 to 15: ```sh bin/reflow main.go 12 15 ``` + +## Go comments + +- Follow [Go Doc Comments](https://go.dev/doc/comment). Write complete + sentences. Start a declaration comment with the name of the symbol. +- Document each exported declaration and its contract. diff --git a/.agents/skills/go-code/SKILL.md b/.agents/skills/go-code/SKILL.md index 374c2e9..59fd544 100644 --- a/.agents/skills/go-code/SKILL.md +++ b/.agents/skills/go-code/SKILL.md @@ -33,6 +33,9 @@ description: Go style rules. Use before you write or review Go code. them. Order functions by first use within their declaration group, not across groups. Entry points do not go before types or constructors merely because they are entry points. +- Group related constants in one `const (...)` block and related + variables in one `var (...)` block. Put an empty line between entries + that have comments. ## Entry point and context @@ -68,6 +71,11 @@ description: Go style rules. Use before you write or review Go code. - Use `errors.Is` and `errors.As` to examine errors. Do not compare an error directly with a sentinel, because errors can be wrapped. `errorlint` checks comparisons and type assertions on errors. +- Do not match the text of an error from another package. When an + external fault leaves no other way, test it through the real code + path, so that a change of the text fails a test. +- Handle a documented sentinel error explicitly when it does not show a + failure. - When a function has more than one error path, wrap each returned error with the context of its operation. A function with one error path can return the error directly. `wrapcheck` checks package and interface @@ -76,7 +84,7 @@ description: Go style rules. Use before you write or review Go code. - When deferred cleanup can fail, use a named `err` result and join the cleanup error with `errors.Join`. -## Logs +## Logs and cancellation - Log with the `*slog.Logger` of the part (`daemonLogger`). The handler formats the line; do not format values into the message. @@ -89,6 +97,12 @@ description: Go style rules. Use before you write or review Go code. - Use info for normal events, warn for a failure that is retried or loses nothing, and error for a failure that loses data or stops a part. +- Cancellation of a service or request context is an expected stop, not + a failure. Do not log it as an error, and do not hide all errors only + because the context is canceled. A joined error with an independent + failure stays a failure. +- Use the worker or request context for this check. An operation that + reaches its own timeout while its worker is active stays a failure. - Never log secrets, tokens, message text, or phone numbers. Redact IDs as the existing code does. @@ -104,14 +118,21 @@ description: Go style rules. Use before you write or review Go code. table-driven test. A small type of two fields that acts as one value, such as a key or a coordinate pair, can use a positional literal on one line. -- Put an empty line between logical blocks and after multiline - declarations, assignments, literals, and calls. The only exception - after a multiline construct is its immediate error check. Separate a - multiline literal from a following loop, even when the loop uses that - literal. Keep a single-line declaration with its immediate use, and an - operation with its immediate error check. This does not waive the - separator after a multiline construct. `wsl_v5` and `nlreturn` check - only part of this rule. Review the other boundaries by hand. +- Put an empty line between logical blocks, such as two independent `if` + statements. Put an empty line before and after loops, switches, + selects, standalone blocks, and multiline declarations, assignments, + literals, and calls, when there is an adjacent statement. These + boundaries also apply when a loop immediately uses the declaration + before it. +- The only exception after a multiline construct is its immediate error + check. Keep a single-line declaration with its immediate use, and an + operation with its immediate error check. A result assertion does not + waive the separator after a multiline construct. `wsl_v5` and + `nlreturn` check only part of these rules. Review the other boundaries + by hand. +- Group imports in this order: standard library, external dependencies, + project imports. Review these groups by hand when the formatter does + not check them. - Use GNU-style command flags: two dashes for long flags, such as `--cwd`, and one dash only for one-letter flags, such as `-h`. - Prefer the Go standard library. Prefer Go over cgo. @@ -127,8 +148,9 @@ description: Go style rules. Use before you write or review Go code. ## Tests -- Add tests for meaningful behavior. Do not add tests that only repeat - implementation details. +- Add tests for behavior changes. Do not add tests that only repeat + implementation details, and do not add a test file without behavior + worth a test. - Use `testify/require` for assertions. `testifylint` checks the assertion forms. - Name a table-driven case slice `testCases` and its range variable @@ -151,4 +173,4 @@ description: Go style rules. Use before you write or review Go code. that more than one test file uses in a file without tests. Name that file for what it holds, and end the name with `_helper_test.go`. - Supply valid required dependencies in tests. Do not make a dependency - optional only to make a test simpler. + optional or add a nil fallback only to make a test simpler.