mirror of
https://github.com/Sea-Haven-Industries/engineering-handbook.git
synced 2026-09-30 23:13:14 +00:00
40553157c2
Replace Changes/Test Plan sections with Validation/Tests/Notes to align with the standardized PR format used across all repos.
55 lines
1.7 KiB
Markdown
55 lines
1.7 KiB
Markdown
# Pull Requests
|
|
|
|
## Scope
|
|
|
|
Each PR should represent a single logical change. If you find yourself writing "and" in the title, consider splitting it into separate PRs.
|
|
|
|
| Good scope | Too broad |
|
|
|---|---|
|
|
| Add receipt parser Lambda | Add receipt parser and refactor auth middleware |
|
|
| Fix timeout in payment processor | Fix timeout and update dependencies |
|
|
| Update Lambda runtime to Python 3.12 | Update runtime and add new endpoint |
|
|
|
|
## Title
|
|
|
|
- Keep it under 70 characters
|
|
- Use imperative mood, same as commit messages
|
|
- Describe the change, not the ticket
|
|
|
|
| Good | Bad |
|
|
|---|---|
|
|
| Add retry logic for transient upstream failures | JIRA-123 |
|
|
| Fix null check in auth handler | Bug fix |
|
|
| Update Lambda runtime to Python 3.12 | Updates |
|
|
|
|
## Description
|
|
|
|
Use a structured format:
|
|
|
|
```markdown
|
|
## Summary
|
|
What changed and why — 1-3 sentences. Explain the motivation, not just the diff.
|
|
|
|
## Validation
|
|
How you verified it works — steps taken, commands run, screenshots if UI.
|
|
|
|
## Tests
|
|
What tests were added, updated, or run. If no automated tests, explain manual testing.
|
|
|
|
## Notes
|
|
Anything reviewers should know — migration steps, deploy order, follow-ups, breaking changes. Omit this section if empty.
|
|
```
|
|
|
|
The summary should explain **why** the change is needed, not just restate the diff. Reviewers can read the code; they need context.
|
|
|
|
## When to Open a PR
|
|
|
|
- Before deploying to production (see [deploy-then-merge](git-workflow.md) workflow)
|
|
- When the work is ready for review, not as a draft for parking incomplete work
|
|
- After verifying locally that the change works as expected
|
|
|
|
## Merging
|
|
|
|
- Squash merge for feature branches with messy interim commits
|
|
- Regular merge for branches with clean, meaningful commit history
|
|
- Delete the branch after merge
|