Share general review and documentation rules

[onechat] / .agents / skills / go-code / SKILL.md

diff --git a/.agents/skills/go-code/SKILL.md b/.agents/skills/go-code/SKILL.md
index 374c2e932d810090dc29af95043f7b20f0490e12..59fd54453a676105491a0bfb95695a26af4afa76 100644 (file)
--- 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.
   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
 
 
 ## 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.
 - 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
 - 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`.
 
 - 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.
 
 - 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.
 - 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.
 
 - 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.
   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.
 - 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
 
 
 ## 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
 - 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
   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.