Chris Farhood
530ecc74e6
- sdlc: trim to application-repo scope with Phase 1-5 pipeline; engineer self-merges all branches with per-branch prerequisites; move infra, Flux, tofu, and operator-install content out - devops: new skill mirroring groombook/org/skills/devops — owns cartsnitch/infra, Flux GitOps, OpenTofu controller, cluster topology, Flux Image Tag Automation denied policy - safety: add Gitea-origin board-approval gate, board-approval scope section, and adapterConfig.env read-before-write rule - coding-standards: replace "no agent merges their own PR" with the reviews-required-then-engineer-may-merge rule consistent with sdlc - CLAUDE.md: update skill index, branch & merge policy, and SDLC phase summary to reflect engineer-self-merge and the new devops skill Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
94 lines
3.9 KiB
Markdown
94 lines
3.9 KiB
Markdown
---
|
|
name: coding-standards
|
|
description: >
|
|
Engineering quality bar for CartSnitch code: priority ordering of correctness
|
|
vs. clarity vs. maintainability vs. performance vs. elegance, PR and test
|
|
requirements, no-hardcoded-values rules, branch discipline, the
|
|
no-self-merge contract, and task-decomposition standards for delegation.
|
|
---
|
|
|
|
# Coding Standards
|
|
|
|
These rules apply to any CartSnitch agent that writes, reviews, or merges code.
|
|
|
|
## Priority ordering
|
|
|
|
When making technical decisions, prioritize in this order:
|
|
|
|
1. **Correctness** — does it work? Does it handle edge cases? Have you proven it, not assumed it?
|
|
2. **Clarity** — will another engineer understand this without context in 6 months?
|
|
3. **Maintainability** — will it be safe to change?
|
|
4. **Performance** — fast enough for the use case? Profile before optimizing.
|
|
5. **Elegance** — nice if free; never trade any of the above for it.
|
|
|
|
## Pull request discipline
|
|
|
|
* All changes go through a PR. **Never push directly to `dev`, `uat`, or `main`.**
|
|
* Never merge a PR without the reviews required by the `sdlc` (or `devops`) skill for that branch. The engineer who opened the PR may click merge once those prerequisites are satisfied.
|
|
* Always include `cc @cpfarhood` at the bottom of the PR body for visibility (never as a reviewer).
|
|
* Engineers always target `dev` — never `uat` or `main` directly.
|
|
|
|
## Test requirements
|
|
|
|
* **Every PR must include tests** for new code paths. No exceptions for "small" changes.
|
|
* Run unit tests, type check, and lint locally (or rely on CI) **before** requesting review.
|
|
* A PR without passing tests does not get approval.
|
|
* New code paths require coverage. No coverage = no approval.
|
|
|
|
## Code review tone
|
|
|
|
Hold a high bar. PRs with obvious mistakes, missing tests, hardcoded values, or policy violations get firm, specific review comments citing what's wrong and what the fix is. Cite the file and line. Suggest the fix when you know it. Don't sugarcoat — but be professional and constructive. "This looks wrong" is not a review comment.
|
|
|
|
## Hardcoded values
|
|
|
|
* **Colors** use CSS variables / theme tokens. Never raw hex in components.
|
|
* **Strings** use constants or i18n. No magic strings.
|
|
* **Numbers** that aren't trivially obvious go in named constants.
|
|
* **No magic numbers** in business logic.
|
|
|
|
## Secrets in code
|
|
|
|
Secrets never touch source. See the `safety` skill for the SealedSecrets workflow. If your implementation requires a Kubernetes secret you cannot create, file an issue for the agent who owns the SealedSecrets workflow rather than committing a plaintext value.
|
|
|
|
## Releases and versioning
|
|
|
|
CartSnitch CI uses **CalVer** (`YYYY.MM.DD[.N]`) for image tags. The CI also publishes `latest` and `sha-<hash>`. Do not introduce other versioning schemes (e.g., custom SemVer for application images). Library packages may use SemVer where the upstream conventions require it.
|
|
|
|
## Container images
|
|
|
|
Push to `git.farh.net/cartsnitch/<service>` only. Never Docker Hub for first-party images.
|
|
|
|
## Task decomposition (for delegators)
|
|
|
|
Every delegated task MUST be structured so the assignee can complete it without ambiguity:
|
|
|
|
* Single, atomic unit of work. If the task requires more than ~3 files to change, split it into multiple tasks.
|
|
* Never delegate tasks requiring architectural judgment or ambiguous scope — make those decisions yourself first, then delegate the concrete action.
|
|
* Include relevant context, examples, or code snippets when the action is non-obvious.
|
|
|
|
### Task description template
|
|
|
|
```
|
|
## What
|
|
[One sentence: the specific action to take]
|
|
|
|
## Where
|
|
[Exact repo, branch, file paths]
|
|
|
|
## Why
|
|
[One sentence: business/technical reason]
|
|
|
|
## How
|
|
[Step-by-step instructions, no ambiguity]
|
|
|
|
## Acceptance Criteria
|
|
- [ ] [Specific, verifiable condition]
|
|
|
|
## Context
|
|
[Code snippets, links, or prior decisions needed to complete the task]
|
|
```
|
|
|
|
## When uncertain
|
|
|
|
If a code-quality call isn't covered above and you can't decide cleanly, escalate to the CTO via comment rather than guessing.
|