mirror of
https://github.com/Sea-Haven-Industries/engineering-handbook.git
synced 2026-09-30 18:33:14 +00:00
0333e7f8f5
Conventions covering naming, git workflow, commit messages, pull requests, code review, GitHub standards, AWS infrastructure, SAM project layout, and secrets management. Commit messages section adapted from RomuloOliveira/commit-messages-guide (CC-BY-4.0).
53 lines
1.5 KiB
Markdown
53 lines
1.5 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
|
|
Brief explanation of what this PR does and why.
|
|
|
|
## Changes
|
|
- Bullet list of specific changes
|
|
|
|
## Test Plan
|
|
- How you verified this works
|
|
- What to check during review
|
|
```
|
|
|
|
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
|