Skip to content

Commit a507c60

Browse files
committed
docs(migration): note two process gaps found while working step 6
- Stacked PR merged into a feature branch that itself never merged forward into migration/effect-http-api (#158/resolve, fixed by #162). - Adversarial review flagging a file as dead based on a same-package grep, missing a cross-package type-only import (#163/admin).
1 parent e6b1d2a commit a507c60

1 file changed

Lines changed: 32 additions & 0 deletions

File tree

docs/migration-effect-http-api-process.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,3 +165,35 @@ that's been silently corrected is easy to repeat:
165165
Doable, but slower and riskier than committing group A immediately
166166
after finishing it, before starting group B -- prefer that ordering
167167
next time even under a "carry on, don't wait for approval" instruction.
168+
- **A stacked PR merged into a feature branch that never itself got merged
169+
forward.** PR #158 (`resolve`) merged cleanly into
170+
`migration/6-profile-group`, exactly per the stacking rule -- but
171+
`migration/6-profile-group`'s tip (with #158 on it) was never merged into
172+
`migration/effect-http-api` itself; the integration branch moved on to
173+
the next steps' docs/client-swap commits instead, so #158 silently never
174+
made it in even though GitHub shows it as merged. Not caught until a
175+
later unrelated task needed to check what was live on
176+
`migration/effect-http-api` and found the old Hono `resolve` files still
177+
on disk, still wired in `app.ts`. The fix was a clean cherry-pick (PR
178+
#162), but the actual lesson: after merging PR B (stacked on PR A's
179+
branch) per the stacking rule, verify PR A's branch tip -- the one B
180+
actually merged into -- is *also* an ancestor of `migration/effect-http-api`,
181+
with the same `git merge-base --is-ancestor` check already used for
182+
branch staleness. Stacking correctly and integrating forward are two
183+
different steps; doing the first is not evidence the second happened.
184+
- **"Nothing else imports this file" is not the same check as "this type
185+
is still consumed."** An adversarial review pass on the `admin` PR (#163)
186+
flagged the old `apps/vps/src/db/admin-overview.schema.ts` (Zod) as fully
187+
dead and safe to delete, based on a grep showing no remaining *runtime*
188+
imports from `apps/vps`. It missed that `apps/www/src/routes/admin/-overview.data.ts`
189+
still imports `AdminOverview`/`AdminOverviewContentBreakdown` as
190+
**types only** from `@gbfm/vps/schemas` -- a cross-package type import
191+
that a same-package runtime-usage grep won't surface unless the check
192+
explicitly greps the consuming app too. The file was correctly left in
193+
place as a type-only shim (to be retired once a 6b PR points
194+
`useAdminOverview` at the new `packages/api` schema's inferred type
195+
instead), but the review's confidence that it was dead was wrong. When a
196+
review claims a file is orphaned, grep the *other* app/package too, not
197+
just the one being edited -- and grep for the exported type names, not
198+
only the schema/const names, since `import type` sites are real
199+
dependencies that a value-only grep pattern can miss.

0 commit comments

Comments
 (0)