- name
- pr-checklist
- description
- Pre-PR verification for imu4helmlog — host tests, device build, clang-format, docs, risk-tier-appropriate checks. TRIGGER when the user is about to open a PR, asks "am I ready to push", or wants a pre-merge review. DO NOT trigger mid-implementation; use /tdd during development.
# Pre-PR checklist
Run these before `gh pr create`. Adjust depth by the PR's risk tier (see
CLAUDE.md). Tier = highest tier of any file touched.
## Always (all tiers)
```bash
pio test -e native # host tests green
pio run -e esp32c3 # device still builds
clang-format --dry-run --Werror $(git ls-files '*.cpp' '*.h')
git status # nothing unintended staged
git diff --stat main...HEAD # scope sanity
```
If any of these fail, stop — don't open the PR.
## Standard tier (sampler, net, config)
- Host tests cover the new behavior — the diff includes `test/test_native/`
changes, not just `lib/`
- New log statements are at appropriate levels (`INFO` for lifecycle, `WARN`
for recoverable, `ERROR` for unrecoverable). No `Serial.println` leftover
- If a config field was added: default value is safe, NVS serialization
round-trips in a test, `docs/protocol.md` (if user-visible) is updated
## High tier (packetizer, uploader, helmlog-side ingest endpoint)
Everything above, plus:
- **Protocol compatibility** — if the wire format changed, `schema` was
bumped, both old and new endpoints coexist in helmlog, and
`docs/protocol.md` + `/protocol` skill reflect the change
- **Round-trip test** — `scripts/fake_device.py` successfully posts to a
local helmlog dev instance; server logs show `accepted: N` matching
the sent count
- **Idempotency** — re-posting the same batch yields `deduped: true`, not
a duplicate row. Verified in integration test if helmlog-side, in unit
test if device-side
- **Retry path** — inject a 500 at the fake sink, verify batch lands in
ringbuf and re-uploads on recovery
## Critical tier (lib/imu/, lib/ringbuf/, protocol schema, mount_quat handling)
Everything above, plus:
- **Peer review of frame math** before merge — a second human reads the
quaternion/transform code. Silent frame bugs corrupt months of data
- **On-device smoke** — `pio test -e esp32c3` runs on a connected board,
I²C enumerates the BNO085, 10 seconds of samples look sane
- **Ringbuf changes**: simulated crash test (kill power mid-write) in the
host test suite recovers without data loss; overflow drops *oldest* and
the newest batch is retained
- **Calibration impact** — if the change affects `mount_quat` storage or
application, `/calibrate` skill is updated and a re-cal procedure is
documented in the PR body
- **Rollback plan** — PR body explicitly states how to roll back (OTA the
prior firmware) and what data would be invalidated if the change shipped
incorrectly
## Cross-repo changes (this repo + helmlog)
If the PR touches the wire protocol, a matching helmlog PR must exist:
- Link both PRs to each other in the body
- Merge **helmlog first** (server accepts new format) then this repo
(device sends new format). Never the other way around — old servers
rejecting new batches means the device fills its ringbuf and eventually
drops data
- Both PRs reference the same issue number in a tracking issue
## PR body template
```markdown
## Summary
- what changed, in 1-3 bullets
- why (not reiterating the diff)
## Risk tier
Critical / High / Standard / Low — [highest-tier file touched]
## Testing
- host tests: <pass/fail, any new test groups added>
- device build: <pass/fail>
- on-device smoke (critical only): <pass/fail, board serial #>
- round-trip (high/critical): <fake_device.py output snippet>
## Rollback
<for critical tier: how to roll back, what data is at risk>
Closes #<issue>
Generated with [Claude Code](https://claude.ai/code)
```
## Things that are NOT checklist items
- "Does it look nice" — that's review, not a checklist
- Coverage percentages — low-signal for embedded; prefer "does a test
exist for this specific branch of behavior"
- Benchmark numbers — unless the PR is performance-motivated, don't
pad the body with microbenchmarks
GitHubで見る