Files

T

Scrubs McBarkley 8930a8d5f1 docs(skills): move uat→main merge-gate policy from coding-standards to sdlc

Reviewer feedback (COrtHvtYnuZx6DmhztGD50uGnKVJajPf): the merge-gate
policy is a process / SDLC rule, not a code-quality / coding-standard
rule, so it belongs in the sdlc skill.

  - skills/sdlc/SKILL.md: add new '## uat→main merge-gate policy'
    section after Phase 5 with the full policy, the three categories,
    the engineer workflow, and the 'when uncertain' escalation path.
    Update frontmatter description and intro paragraph to point at
    the new local section. Re-point the branch-strategy table row
    and Phase 4 step 3 at the local section.
  - skills/coding-standards/SKILL.md: remove the duplicate
    'uat→main merge-gate policy' section (it now lives in sdlc) and
    replace it with a one-paragraph pointer to sdlc. Update the
    frontmatter description to remove the policy bullet and add a
    'lives in sdlc, not here' line.

No behavior change: the policy content is identical, only its home
file moved. The PR is now an sdlc PR with a small coding-standards
follow-on, which matches the reviewer's point.

Refs: GRO-2377
Triggers: GRO-2358, GRO-2359
Source rule: GRO-2348 (merge-whitelist fix)

Co-Authored-By: Paperclip <noreply@paperclip.ing>

2026-06-12 01:39:27 +00:00

3.4 KiB

Raw Blame History

name, description

name	description
coding-standards	Engineering quality bar for GroomBook code: priority ordering of correctness vs. clarity vs. maintainability vs. performance vs. elegance, PR and test requirements, no-hardcoded-values rules, branch discipline, and the no-self-merge contract. The uat→main merge-gate policy lives in the `sdlc` skill, not here.

Coding Standards

These rules apply to any GroomBook agent that writes, reviews, or merges code.

Priority ordering

When making technical decisions, prioritize in this order:

Correctness — does it work? Does it handle edge cases? Have you proven it, not assumed it?
Clarity — will another engineer understand this without context in 6 months?
Maintainability — will it be safe to change?
Performance — fast enough for the use case? Profile before optimizing.
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 (not as a reviewer).

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

All releases use CalVer (YYYY.MMDD.PATCH, e.g. 2026.0504.0). No SemVer, no custom schemes.

Container images

Push to git.farh.net only. Never Docker Hub for first-party images.

uat→main merge-gate policy

The uat→main merge-gate policy lives in the sdlc skill, not here. The one-line summary: the engineer self-merges a uat→main PR once the four pre-gates (QA, UAT deploy, UAT regression, security) are green and the CEO code review is APPROVED on the Paperclip issue. A CTO Gitea Approve click is reserved for three categories: novel auth / session paths, infra / prod-affecting merges, and risk-flagged merges. See the sdlc skill — "uat→main merge-gate policy" — for the full rule, the category list, and the "when uncertain" escalation path.

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.

3.4 KiB Raw Blame History