mirror of
https://github.com/Sea-Haven-Industries/engineering-handbook.git
synced 2026-10-07 16:19:06 +00:00
Compare commits
No commits in common. "0e83835b4a03e037e561ff555c8493a857439416" and "9c65fcb053d99ef6f5b2b36992bf144e67fb2002" have entirely different histories.
0e83835b4a
...
9c65fcb053
4 changed files with 35 additions and 87 deletions
|
|
@ -11,7 +11,7 @@ Engineering conventions and best practices for Sea Haven Industries.
|
|||
- [Naming Conventions](naming-conventions.md) -- kebab-case everywhere, no exceptions
|
||||
- [Development Environment](dev-environment.md) -- workstation directory layout, pyenv, Node, launchd/TCC
|
||||
- [Git Workflow](git-workflow.md) -- feature branches, incremental commits, deploy-then-merge
|
||||
- [Commit Messages](commit-messages.md) -- Conventional Commits, type(scope) format, explain "why"
|
||||
- [Commit Messages](commit-messages.md) -- imperative mood, 50/72 rule, explain "why"
|
||||
- [Pull Requests](pull-requests.md) -- scope, title, description format, merge strategy
|
||||
- [Code Review](code-review.md) -- what to look for, giving feedback, turnaround expectations
|
||||
- [Code Review Rubric](code-review-rubric.md) -- BLOCK/FIX/NIT/QUESTION finding categories and output format
|
||||
|
|
|
|||
|
|
@ -2,111 +2,65 @@
|
|||
|
||||
> Adapted from [commit-messages-guide](https://github.com/RomuloOliveira/commit-messages-guide) by Romulo Oliveira, licensed under [CC-BY-4.0](https://creativecommons.org/licenses/by/4.0/).
|
||||
|
||||
Sea Haven uses [Conventional Commits](https://www.conventionalcommits.org/) for every commit and PR title. A structured `type(scope): description` header keeps history scannable, drives tooling (changelogs, release tagging, PR-title linting), and maps cleanly onto [SemVer](git-workflow.md#versioning).
|
||||
|
||||
## Format
|
||||
|
||||
```
|
||||
type(scope): description
|
||||
```
|
||||
|
||||
- **type** (required): the category of change, from the [Types](#types) table below.
|
||||
- **scope** (optional): the area of the codebase affected, in parentheses and lowercase (`auth`, `api`, `parser`, `deps`). Omit it when the change is broad or no single scope fits.
|
||||
- **description** (required): a short summary in imperative mood, lowercase, with no trailing period.
|
||||
|
||||
Examples:
|
||||
|
||||
```
|
||||
feat(parser): add retry logic for transient upstream failures
|
||||
fix(auth): correct null check in session handler
|
||||
docs: document the deploy-then-merge workflow
|
||||
chore(deps): bump aws-cdk-lib to 2.150.0
|
||||
```
|
||||
|
||||
## Types
|
||||
|
||||
| Type | Use for |
|
||||
|---|---|
|
||||
| `feat` | A new feature or capability |
|
||||
| `fix` | A bug fix |
|
||||
| `docs` | Documentation only |
|
||||
| `style` | Formatting or whitespace with no behavior change |
|
||||
| `refactor` | A code change that neither fixes a bug nor adds a feature |
|
||||
| `perf` | A change that improves performance |
|
||||
| `test` | Adding or correcting tests |
|
||||
| `build` | Build system, packaging, or dependency changes |
|
||||
| `ci` | CI/CD configuration and workflows |
|
||||
| `chore` | Routine maintenance that doesn't touch source or tests |
|
||||
| `revert` | Reverting a previous commit |
|
||||
| `release` | Cutting a release (version bump, tag, changelog) |
|
||||
|
||||
## Rules
|
||||
|
||||
### Keep the header short
|
||||
|
||||
Keep the whole header (type, scope, and description) under 72 characters, and aim to keep the description itself around 50. The header is for scanning; details go in the body.
|
||||
|
||||
### Use imperative mood in the description
|
||||
### Use imperative mood
|
||||
|
||||
Write what the commit *does*, not what you *did*.
|
||||
|
||||
| Good | Bad |
|
||||
|---|---|
|
||||
| `feat: add retry logic to payment processor` | `feat: added retry logic to payment processor` |
|
||||
| `fix: correct null check in auth handler` | `fix: fixed null check in auth handler` |
|
||||
| `refactor: remove deprecated endpoint` | `refactor: removing deprecated endpoint` |
|
||||
| Add retry logic to payment processor | Added retry logic to payment processor |
|
||||
| Fix null check in auth handler | Fixed null check in auth handler |
|
||||
| Remove deprecated endpoint | Removing deprecated endpoint |
|
||||
|
||||
### Lowercase the description, no trailing period
|
||||
|
||||
The type carries the emphasis, so the description stays lowercase and unpunctuated.
|
||||
### Capitalize the first letter
|
||||
|
||||
| Good | Bad |
|
||||
|---|---|
|
||||
| `fix: correct null check in auth handler` | `fix: Correct null check in auth handler.` |
|
||||
| Fix null check in auth handler | fix null check in auth handler |
|
||||
|
||||
### Keep the subject line under 50 characters
|
||||
|
||||
The subject line is for scanning. Details go in the body.
|
||||
|
||||
### Wrap the body at 72 characters
|
||||
|
||||
If you need to explain more, add a blank line after the subject and write a body wrapped at 72 characters per line.
|
||||
|
||||
### Explain "why," not "what"
|
||||
|
||||
The diff shows what changed. The body should explain why.
|
||||
The diff shows what changed. The commit message should explain why.
|
||||
|
||||
| Good | Bad |
|
||||
|---|---|
|
||||
| `perf: increase timeout to handle slow upstream responses` | `perf: change timeout from 30 to 60` |
|
||||
| `build: switch to arm64 to reduce Lambda cold start cost` | `build: update architecture setting` |
|
||||
| Increase timeout to handle slow upstream responses | Change timeout from 30 to 60 |
|
||||
| Switch to arm64 to reduce Lambda cold start cost | Update architecture setting |
|
||||
|
||||
### No generic descriptions
|
||||
### No generic messages
|
||||
|
||||
A valid type does not excuse a vague description. These are not acceptable:
|
||||
These are not acceptable:
|
||||
|
||||
- `fix: stuff`
|
||||
- `chore: update code`
|
||||
- `chore: misc changes`
|
||||
- `chore: address review comments`
|
||||
- `Fix stuff`
|
||||
- `Update code`
|
||||
- `WIP`
|
||||
- `Misc changes`
|
||||
- `Address review comments`
|
||||
|
||||
Every commit should describe a specific change. `WIP` commits do not belong on shared branches.
|
||||
Every commit should describe a specific change.
|
||||
|
||||
### No self-referential language
|
||||
|
||||
| Good | Bad |
|
||||
|---|---|
|
||||
| `fix: correct race condition in queue processor` | `fix: this commit fixes a race condition` |
|
||||
| `feat: add input validation for email field` | `feat: this PR adds validation` |
|
||||
| `refactor: simplify auth middleware for clarity` | `refactor: I refactored the auth middleware` |
|
||||
|
||||
## Breaking changes
|
||||
|
||||
Mark a breaking change with a `!` before the colon, and add a `BREAKING CHANGE:` footer describing the migration. Breaking changes drive a SemVer MAJOR bump (see [Versioning](git-workflow.md#versioning)).
|
||||
|
||||
```
|
||||
feat(api)!: remove the deprecated /v1/receipts endpoint
|
||||
|
||||
BREAKING CHANGE: clients must migrate to /v2/receipts. The v1
|
||||
response shape is no longer returned.
|
||||
```
|
||||
| Fix race condition in queue processor | This commit fixes a race condition |
|
||||
| Add input validation for email field | This PR adds validation |
|
||||
| Refactor auth middleware for clarity | I refactored the auth middleware |
|
||||
|
||||
## Template
|
||||
|
||||
```
|
||||
type(scope): description (header under 72 chars, imperative, lowercase)
|
||||
Subject line (50 chars max, imperative, capitalized)
|
||||
|
||||
Optional body wrapped at 72 characters. Explain the problem
|
||||
this commit solves and why this approach was chosen. Mention
|
||||
|
|
@ -127,7 +81,7 @@ List both when both apply: `Refs: PROJ-123, #123`. The key only needs to appear
|
|||
## Example
|
||||
|
||||
```
|
||||
feat(payments): add retry logic for transient upstream failures
|
||||
Add retry logic for transient upstream failures
|
||||
|
||||
The payment processor occasionally returns 503 during
|
||||
deployments. Without retries, these surface as user-facing
|
||||
|
|
|
|||
|
|
@ -11,12 +11,6 @@
|
|||
| `feature/<key>-<description>` | Adding new functionality or enhancing existing features |
|
||||
| `bug/<key>-<description>` | Fixing a non-urgent defect found during development or testing |
|
||||
| `hotfix/<key>-<description>` | Fixing a production issue that needs immediate attention |
|
||||
| `chore/<key>-<description>` | Routine maintenance, cleanup, dependency bumps, or tooling |
|
||||
| `docs/<key>-<description>` | Documentation-only changes |
|
||||
| `refactor/<key>-<description>` | Restructuring code without changing behavior |
|
||||
| `release/<key>-<description>` | Preparing a release (version bump, tag, changelog) |
|
||||
|
||||
The prefixes mirror the commit types in [commit-messages.md](commit-messages.md#types), so the branch and its commits speak the same vocabulary.
|
||||
|
||||
**bug vs hotfix:** Use `bug/` for defects caught before they affect production (failing tests, broken dev flows, issues found in review). Use `hotfix/` only when production is impacted and the fix needs to bypass normal review cadence.
|
||||
|
||||
|
|
|
|||
|
|
@ -13,14 +13,14 @@ Each PR should represent a single logical change. If you find yourself writing "
|
|||
## Title
|
||||
|
||||
- Keep it under 70 characters
|
||||
- Use the Conventional Commit format, same as commit messages (`type(scope): description`)
|
||||
- Use imperative mood, same as commit messages
|
||||
- Describe the change, not the ticket
|
||||
|
||||
| Good | Bad |
|
||||
|---|---|
|
||||
| `feat: add retry logic for transient upstream failures` | `JIRA-123` |
|
||||
| `fix: correct null check in auth handler` | `Bug fix` |
|
||||
| `build: update Lambda runtime to Python 3.12` | `Updates` |
|
||||
| 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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue