Michael Freno
e8d00bde6f
- Add Gitea PR template with code review checklist - Add CODEOWNERS file for review assignment policy - Update shieldai-workflow.md with implemented workflow - Add branch-protection-rules.yaml for gt/master protection Co-Authored-By: Paperclip <noreply@paperclip.ing>
84 lines
2.7 KiB
Markdown
84 lines
2.7 KiB
Markdown
# ShieldAI Code Review Workflow
|
|
|
|
## Current State (as of May 2, 2026)
|
|
|
|
### PR Backlog Status
|
|
- **Open PRs**: 0 (pending commits pushed to master)
|
|
- **Pending commits**: 1 commit pushed (FRE-4604) — remaining 6 were previously pushed
|
|
- **Last review cycle**: FRE-4500, FRE-4499, FRE-4612 (security findings — all done)
|
|
- **Branch protection**: Configured (see `branch-protection-rules.yaml`)
|
|
- **PR template**: Configured (`.gitea/pull_request_templates/default.md`)
|
|
|
|
### Resolved Bottlenecks
|
|
1. ✅ PR-based workflow established with PR template
|
|
2. ✅ Branch protection rules documented and configured
|
|
3. ✅ Code review checklist integrated into PR template
|
|
4. ✅ Security review findings integrated (FRE-4499, FRE-4500, FRE-4612 all done)
|
|
|
|
## PR Process
|
|
|
|
1. **Feature branch creation** from `gt/master`
|
|
2. **Development commits** with conventional commit format (include issue ID: `FRE-XXXX: description`)
|
|
3. **PR creation** against `gt/master`
|
|
4. **Required reviews**:
|
|
- Code Reviewer — all PRs
|
|
- Security Reviewer — for security-sensitive changes
|
|
5. **CI checks** pass (lint, typecheck, test)
|
|
6. **Merge** via squash or rebase
|
|
|
|
### Code Review Checklist
|
|
|
|
- [ ] Security impact assessment
|
|
- [ ] Test coverage verification
|
|
- [ ] Type checking (TypeScript)
|
|
- [ ] Linting compliance
|
|
- [ ] Documentation updates
|
|
- [ ] Breaking changes documented
|
|
- [ ] Backward compatibility verified
|
|
|
|
### Branch Protection Rules
|
|
|
|
See `branch-protection-rules.yaml` for the full configuration. Summary:
|
|
|
|
- **Protected branch**: `gt/master`
|
|
- **Required reviews**: 1 approved review before merge
|
|
- **Required status checks**: lint, typecheck, test
|
|
- **Enforce admins**: false (admins can bypass during emergencies)
|
|
- **Allow force pushes**: true (for recovery scenarios)
|
|
|
|
## Review Assignment Policy
|
|
|
|
| Change Type | Required Reviewers |
|
|
|-------------|-------------------|
|
|
| General code | Code Reviewer |
|
|
| Security-critical | Code Reviewer + Security Reviewer |
|
|
| API contracts | Code Reviewer + CTO |
|
|
| Database schema | Code Reviewer + Senior Engineer |
|
|
|
|
## Review Pipeline
|
|
|
|
```
|
|
Engineer implements → marks in_review → Security Reviewer reviews → Code Reviewer reviews → Done
|
|
```
|
|
|
|
## Metrics to Track
|
|
|
|
- PR cycle time (creation to merge)
|
|
- Review turnaround time
|
|
- PR size (lines changed)
|
|
- Review comments per PR
|
|
- Merge conflict frequency
|
|
|
|
## Contribution Guidelines
|
|
|
|
1. Always create a feature branch from `gt/master`
|
|
2. Use conventional commit format: `type(scope): description (FRE-XXXX)`
|
|
3. Include tests for new functionality
|
|
4. Update documentation for API changes
|
|
5. Run lint and typecheck before pushing
|
|
6. Create PR with filled template before requesting review
|
|
7. Address all review comments before merge
|
|
|
|
---
|
|
*Updated from FRE-4556 audit, implemented in FRE-4661*
|