Files
review-bot/CONVENTIONS.md
Rodin 0619e2b617 fix: address review feedback on check-deps script
- Accept import paths starting with digit (e.g., 9fans.net/go) by
  relaxing filter from ^[a-zA-Z] to ^[[:alnum:]]
- Fix misleading comment: loop uses Bash process substitution, not POSIX
- Remove redundant \$ from grep regex (literal $ unreachable)
- Capture stderr separately in go list to avoid mixing errors with output
- Clarify CONVENTIONS.md wording on transitive vs direct deps

Reviewed-by: sonnet-review-bot
Reviewed-by: gpt-review-bot
2026-05-10 14:09:03 -07:00

51 lines
1.6 KiB
Markdown

# Conventions
## Language & Dependencies
- Target the latest stable Go release.
- **STRICT ALLOWLIST:** Only packages listed below may be imported. No exceptions.
### Approved Third-Party Packages
| Package | Use Case | Scope |
|---------|----------|-------|
| `gopkg.in/yaml.v3` | YAML parsing (persona files, config) | production |
| `github.com/google/go-cmp` | Test comparisons (`cmp.Diff`) | test only |
**Any import not in this table or the Go standard library is forbidden.**
Only *direct* dependencies (listed in go.mod without `// indirect`) are checked against this allowlist. Transitive dependencies pulled in by approved packages are implicitly allowed.
To request a new dependency:
1. Open a PR that ONLY updates this table
2. Requires explicit approval from Aaron
3. After merge, a separate PR may use the package
*Enforcement: `scripts/check-deps.sh` parses this table — update only here.*
## Error Handling
- Return errors; never panic.
- Wrap errors with context using `fmt.Errorf("context: %w", err)`.
- Check all error returns.
## Testing
- Test every exported function.
- Use `net/http/httptest` for HTTP mocking.
- Table-driven tests where multiple inputs share the same assertion logic.
- Integration tests use build tags (`//go:build integration`).
## Style
- Keep functions short and focused.
- Prefer early returns over deep nesting.
- Meaningful variable names — no single-letter names outside loop indices.
- Comments explain *why*, not *what*.
## Process
- `go test ./...` must pass before commit.
- `go vet ./...` must pass before commit.
- Keep commits atomic and well-described.