Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- **CLI error messages must name the resource.** When a CLI command fails for a named resource (task, spawner, workspace, etc.), include the resource name in the returned error so operators piping or batching invocations get an actionable signal. `fmt.Errorf("task %s failed", name)`, not `errors.New("task failed")`.
- **Test the happy path, not only the early-return guards.** When a handler has both early-return guards and a primary action (post a message, create a resource, emit a metric), unit tests must include at least one positive case verifying the primary action runs with the right arguments β€” not just the no-op branches. If the production code lacks a seam, add one (interface, function field, fake client) so the happy path is covered.
- **Avoid vacuous substring assertions in printer/formatter tests.** When asserting a `label: value` line is emitted, match against the full `"label: value"` string (or a regex), not the bare value β€” bare values often collide with the resource's `Name` or other surrounding context in the fixture and pass even when the line is missing.
- **Assert byte-equality, not `Contains`, in stream-forwarding tests.** When a function both forwards bytes from an input to an output and parses metadata along the way (a `StreamUsage`-style tee/parse loop), assert that the full captured output equals the normalized expected bytes (`if out.String() != want`), not `strings.Contains(out, line+"\n")` per input line. A per-line substring check still passes when the forwarder drops a partial line, reorders writes, or duplicates output β€” exactly the regressions a streaming refactor risks. If the same package already has a stricter byte-equality test (e.g., for unterminated input), reuse its shape across the table-driven cases instead of weakening to `Contains`.
- **Keep CRD enum docstrings consistent with the `+kubebuilder:validation:Enum` marker.** If the godoc says "empty matches both", either include `""` in the enum list so `field: ""` is accepted, or rephrase to "Omit to match both" so no one writes the explicit empty form. A docstring that invites a value the API server then rejects is a worse contract than either alternative.
- **Qualify cross-CRD field references with the owning kind in docs.** In a CRD reference section, write `Task.spec.podOverrides.env` rather than bare `podOverrides.env` when describing a field that lives on a sibling CRD. A reader of one CRD's reference page should be able to locate the cited field without already knowing the layout of the others.

Expand Down
1 change: 1 addition & 0 deletions self-development/agentconfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ spec:
- CLI error messages must name the resource: when a CLI command fails for a named resource (task, spawner, workspace, etc.), include the name in the returned error so operators piping or batching invocations get an actionable signal β€” `fmt.Errorf("task %s failed", name)`, not `errors.New("task failed")`.
- Test the happy path, not only early-return guards: when a handler has both guard branches and a primary action (post a message, create a resource, emit a metric), include at least one positive test verifying the primary action runs with the right arguments. If the production code lacks a seam, add one (interface, function field, fake client) so the happy path is testable.
- Avoid vacuous substring assertions in printer/formatter tests: when asserting a `label: value` line is emitted, match the full `"label: value"` string (or a regex), not the bare value β€” bare values frequently collide with the fixture's `Name` or surrounding context and pass even when the line is missing.
- Assert byte-equality, not `Contains`, in stream-forwarding tests: when a function both forwards bytes from an input to an output and parses metadata along the way (a `StreamUsage`-style tee/parse loop), assert `out.String() == want` against the normalized expected bytes, not `strings.Contains(out, line+"\n")` per input line. A per-line substring check still passes when the forwarder drops a partial line, reorders writes, or duplicates output β€” exactly the regressions a streaming refactor risks. If the same package already has a stricter byte-equality test (e.g., for unterminated input), reuse its shape across the table-driven cases instead of weakening to `Contains`.
- Keep CRD enum docstrings consistent with the `+kubebuilder:validation:Enum` marker: if the godoc says "empty matches both", either include `""` in the enum list so `field: ""` is accepted, or rephrase to "Omit to match both" so no one writes the explicit empty form. A docstring that invites a value the API server then rejects is a worse contract than either alternative.
- Qualify cross-CRD field references with the owning kind in docs: in a CRD reference section, write `Task.spec.podOverrides.env` rather than bare `podOverrides.env` when describing a field that lives on a sibling CRD. A reader of one CRD's reference page should be able to locate the cited field without already knowing the layout of the others.
- PRs that only modify files under `self-development/` are internal agent improvements: use `/kind cleanup` and write "NONE" in the `release-note` block, even when the change fixes a bug or adds a feature in agent behavior. Classify by file location, not by problem nature.
Expand Down
1 change: 1 addition & 0 deletions self-development/kelos-workers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ spec:
- CLI error messages must name the resource: when a CLI command fails for a named resource (task, spawner, workspace, etc.), include the name in the returned error so operators piping or batching invocations get an actionable signal β€” `fmt.Errorf("task %s failed", name)`, not `errors.New("task failed")`.
- Test the happy path, not only early-return guards: when a handler has both guard branches and a primary action (post a message, create a resource, emit a metric), include at least one positive test verifying the primary action runs with the right arguments. If the production code lacks a seam, add one (interface, function field, fake client) so the happy path is testable.
- Avoid vacuous substring assertions in printer/formatter tests: when asserting a `label: value` line is emitted, match the full `"label: value"` string (or a regex), not the bare value β€” bare values frequently collide with the fixture's `Name` or surrounding context and pass even when the line is missing.
- Assert byte-equality, not `Contains`, in stream-forwarding tests: when a function both forwards bytes from an input to an output and parses metadata along the way (a `StreamUsage`-style tee/parse loop), assert `out.String() == want` against the normalized expected bytes, not `strings.Contains(out, line+"\n")` per input line. A per-line substring check still passes when the forwarder drops a partial line, reorders writes, or duplicates output β€” exactly the regressions a streaming refactor risks. If the same package already has a stricter byte-equality test (e.g., for unterminated input), reuse its shape across the table-driven cases instead of weakening to `Contains`.
- Keep CRD enum docstrings consistent with the `+kubebuilder:validation:Enum` marker: if the godoc says "empty matches both", either include `""` in the enum list so `field: ""` is accepted, or rephrase to "Omit to match both" so no one writes the explicit empty form. A docstring that invites a value the API server then rejects is a worse contract than either alternative.
- Qualify cross-CRD field references with the owning kind in docs: in a CRD reference section, write `Task.spec.podOverrides.env` rather than bare `podOverrides.env` when describing a field that lives on a sibling CRD. A reader of one CRD's reference page should be able to locate the cited field without already knowing the layout of the others.
- PRs that only modify files under `self-development/` are internal agent improvements: use `/kind cleanup` and write "NONE" in the `release-note` block, even when the change fixes a bug or adds a feature in agent behavior. Classify by file location, not by problem nature.
Expand Down
Loading