konflux-ci/integration-service/pr-definition-of-done

PR Definition of Done

Overview

Every PR must pass CI checks, follow commit conventions, include tests, and keep generated code in sync. This checklist covers what CI enforces and what reviewers expect.

When to Use

• Before pushing a PR for review
• When CI checks fail and you need to understand why
• When reviewing someone else's PR

Pre-Push Checklist

Commits

• [ ] Conventional format: type(JIRA-ID): description (e.g., feat(STONEINTG-1519): create PR group snapshots)
• [ ] Types: feat, fix, chore, refactor, test, docs
• [ ] Signed off (DCO): git commit -s
• [ ] Title < 72 chars, description lines < 72 chars
• [ ] AI-assisted work: add Assisted-by: <tool-name> trailer

Code Generation (CI will fail if stale — see ci-cd-quirks for details)

• [ ] After API type changes in api/v1beta2/: make generate manifests
• [ ] After dependency changes: go mod tidy && go mod vendor
• [ ] Run make fmt — CI fails on uncommitted formatting changes
• [ ] Run make vet — catches common Go mistakes

Testing

• [ ] Unit tests alongside code changes (Ginkgo + envtest)
• [ ] Cover: happy path, error conditions, edge cases, idempotency
• [ ] make test passes locally
• [ ] Changed coverable lines meet the 85% patch-coverage target (run estimating-codecov-patch-coverage)

Security & RBAC

• [ ] No secrets, keys, or credentials committed
• [ ] No RBAC wildcards in config/ (CI checks this)
• [ ] New resource types: add +kubebuilder:rbac markers
• [ ] All cached resources accessed via client.Get() or client.List() have list and watch verbs in RBAC markers — excludes DisableFor resources (see ci-cd-quirks)

Documentation

• [ ] PR description explains the "why", not just the "what"
• [ ] PR title < 72 chars
• [ ] Controller diagram in docs/ updated if controller logic changed

What CI Checks

Common Mistakes