Guidelines for submitting and reviewing pull requests.
Before Submitting
Pre-PR Checklist
-
Code compiles without warnings:
cargo build -
All tests pass:
cargo test -
Code is formatted:
cargo fmt -
Lints pass:
cargo clippy --workspace --all-targets -- -D warnings - Documentation updated if needed
- Changelog updated for user-facing changes
- Commit messages follow conventions
Branch Preparation
# Sync with upstream
git fetch upstream
git rebase upstream/main
# Squash WIP commits
git rebase -i upstream/main
# Mark commits as 'squash' or 'fixup'
# Force push to update branch
git push --force-with-lease
Creating a Pull Request
PR Title Format
<type>: <description>
Examples:
feat: add circuit breaker for upstream connections
fix: resolve memory leak in WebSocket handler
docs: update agent development guide
refactor: simplify config parsing logic
perf: optimize route matching algorithm
test: add integration tests for health checks
chore: update dependencies
PR Description Template
## Summary
Brief description of what this PR does.
## Changes
- Added X functionality
- Fixed Y bug
- Updated Z documentation
## Testing
Describe how you tested these changes:
- Unit tests added/updated
- Manual testing performed
- Integration tests run
## Related Issues
Closes #123
Related to #456
## Checklist
- [ ] Tests added/updated
- [ ] Documentation updated
- [ ] Changelog entry added
- [ ] Breaking changes documented
PR Size Guidelines
Ideal PR Size
| Type | Lines Changed | Files |
|---|---|---|
| Bug fix | < 100 | 1-3 |
| Feature | < 500 | 3-10 |
| Refactor | < 300 | 5-15 |
Large PRs
If your PR is large:
- Consider splitting into smaller PRs
- Add detailed description explaining the scope
- Use a feature branch with incremental commits
- Request multiple reviewers
Stacked PRs
For large features:
main
└── feature/base-infrastructure (PR #1)
└── feature/core-logic (PR #2)
└── feature/api-integration (PR #3)
Review Process
Getting Reviews
- Request Reviewers: Add relevant team members
- Add Labels:
needs-review, feature area labels - Link Issues: Reference related issues
- CI Must Pass: All checks must be green
Review Timeline
| Priority | Target Response |
|---|---|
| Critical (security) | Same day |
| High (blocking) | 1 day |
| Normal | 2-3 days |
| Low (docs, chores) | 1 week |
Review States
| State | Meaning |
|---|---|
| Approved | Ready to merge |
| Changes Requested | Must address feedback |
| Commented | Non-blocking feedback |
Responding to Reviews
Addressing Feedback
# Make requested changes
git add .
git commit -m "fix: address review feedback"
# Push updates
git push
# Re-request review
Discussing Feedback
- Respond to all comments
- Explain your reasoning if you disagree
- Mark conversations as resolved when addressed
- Use “Resolve conversation” button
Common Review Patterns
Nit - Minor suggestion, non-blocking:
nit: Consider renaming this variable for clarity
Question - Seeking clarification:
Q: Why did you choose this approach over X?
Suggestion - Proposed change:
suggestion: This could be simplified:
\`\`\`rust
let result = items.iter().filter(|x| x.is_valid()).count();
\`\`\`
Merge Requirements
Required Checks
The following CI checks must pass (matching .github/workflows/ci.yml):
- Formatting —
cargo fmt --all --check - Clippy —
cargo clippy --workspace --all-targets --all-features -- -D warnings - Tests —
cargo test --workspace - Documentation —
cargo doc --workspace --no-depswith-D warnings
Additionally:
- At least one approval
- No unresolved conversations
- Branch is up to date with main
Merge Strategy
Use Squash and Merge for most PRs:
feat: add circuit breaker (#123)
- Implement failure counting
- Add configurable threshold
- Include metrics
Co-authored-by: Reviewer <[email protected]>
Use Rebase and Merge for:
- Multiple logical commits that should be preserved
- Large features with meaningful history
After Merge
- Delete the feature branch
- Verify deployment (if applicable)
- Close related issues
- Update documentation if needed
Special Cases
Draft PRs
Use draft PRs for:
- Work in progress needing early feedback
- Experiments
- RFC-style discussions
# Create PR as draft via CLI
gh pr create --draft
Breaking Changes
For breaking changes:
- Add
breakinglabel - Document migration path in PR description
- Update CHANGELOG with
### BREAKING - Consider deprecation period
## BREAKING CHANGES
This PR changes the configuration format for upstreams.
### Migration
Before:
\`\`\`kdl
upstream "backend" {
address "127.0.0.1:3000"
}
\`\`\`
After:
\`\`\`kdl
upstream "backend" {
targets {
target { address "127.0.0.1:3000" }
}
}
\`\`\`
Security Fixes
For security-related changes:
- Do NOT include exploit details in public PR
- Coordinate with maintainers privately
- Prepare security advisory
- Merge and release together
Reverts
If a change needs to be reverted:
git revert <commit-hash>
git push origin revert-branch
# Create PR with explanation
gh pr create --title "revert: <original title>" \
--body "Reverts #123 due to <reason>"
PR Hygiene
Keep PRs Current
# Update branch regularly
git fetch upstream
git rebase upstream/main
git push --force-with-lease
Close Stale PRs
PRs inactive for 30+ days may be closed. To reopen:
- Rebase on current main
- Address any outdated feedback
- Re-request review
PR Labels
| Label | Meaning |
|---|---|
needs-review | Ready for review |
wip | Work in progress |
breaking | Contains breaking changes |
security | Security-related |
docs | Documentation only |
good-first-issue | Good for newcomers |
GitHub CLI Commands
# Create PR
gh pr create
# Check PR status
gh pr status
# View PR
gh pr view 123
# Checkout PR locally
gh pr checkout 123
# Merge PR
gh pr merge 123 --squash
# Request review
gh pr edit 123 --add-reviewer username
Next Steps
- Contributing - Contribution guidelines
- Code Style - Formatting conventions
- Release Process - How releases work