| # Definition of Done (PR Checklist) |
|
|
| ## Automated ✅ (CI/GitHub enforces these) |
| These run automatically via `ci.yml`. If your PR fails them, fix and re-push. |
| - [ ] Go formatting & linting passes (gofmt, vet, golangci-lint) |
| - [ ] Unit + E2E tests pass (`go test ./...` and `./dev e2e`) |
| - [ ] Build succeeds (`go build`) |
| - [ ] CodeQL security scan passes |
| - [ ] Branch naming follows convention |
|
|
| ## Manual — Code Quality (Required) |
| - [ ] **Error handling explicit** — All errors wrapped with `%w`, no silent failures |
| - [ ] **No regressions** — Verify stealth, token efficiency, session persistence work (test locally if unsure) |
| - [ ] **SOLID principles** — Functions do one thing, testable, no unnecessary deps |
| - [ ] **No redundant comments** — Comments explain *why* or *context*, not *what* the code does |
| - ❌ Bad: `// Loop through items` above `for _, item := range items` |
| - ❌ Bad: `// Return error` above `return err` |
| - ✅ Good: `// Prioritize chromium-browser on ARM64 (Raspberry Pi default)` |
| - ✅ Good: `// Chrome may not be installed in CI, so empty result is valid` |
|
|
| ## Manual — Testing (Required) |
| - [ ] **New/changed functionality has tests** — Same-package unit tests preferred |
| - [ ] **Docker E2E tests run locally** — If you modified handlers/bridge/tabs, run: `./dev e2e` (Docker curl E2E) and/or `./dev e2e cli` (Docker CLI E2E) |
| - [ ] **npm commands work** (if npm wrapper touched): |
| - `npm pack` in `/npm/` produces valid tarball |
| - `npm install -g pinchtab` (or from local tarball) succeeds |
| - `pinchtab --version` + basic commands work after install |
|
|
| ## Manual — Documentation (Required) |
| - [ ] **README.md updated** — If user-facing changes (CLI, API, env vars, install) |
| - [ ] **/docs/ updated** — If API/architecture/perf changed (optional for small fixes) |
|
|
| ## Manual — Review (Required) |
| - [ ] **PR description explains what + why** — Especially stealth/perf/compatibility impact |
| - [ ] **Commits are atomic** — Logical grouping, good messages |
| - [ ] **No breaking changes to npm** — Unless explicitly major version bump |
|
|
| ## Conditional (Only if applicable) |
| - [ ] Headed-mode tested (if dashboard/UI changes) |
| - [ ] Breaking changes documented in PR description (if any) |
|
|
| --- |
|
|
| ## Quick Checklist (Copy/Paste for PRs) |
| ```markdown |
| ## Definition of Done |
| - [ ] Unit + Docker E2E tests added & passing |
| - [ ] Error handling explicit (wrapped with %w) |
| - [ ] No regressions in stealth/perf/persistence |
| - [ ] No redundant comments (explain why, not what) |
| - [ ] README/docs updated (if user-facing) |
| - [ ] npm install works (if npm changes) |
| ``` |
|
|
