docs(#560): re-fork & data-preservation guide in upgrading.md#561
Conversation
- Add "When upgrading isn't enough" section to docs/upgrading.md: the data-vs-framework mental model, when to re-fork vs /update, preserving portfolio data (split-portfolio first, manual fallback), and the "project tickets filed against the framework repo" trap. - Link upgrading.md from README (was only linked from multi-project.md). Extends the existing /update-mechanics doc rather than adding a new file, to avoid doc fragmentation. Refs #560
atlas-apex
left a comment
There was a problem hiding this comment.
Code Review: PR #561
Commit: 09db8766ab6df227fd1b1990d2f972342a040954
Summary
Docs-only PR (2 files, +87/-0). Extends docs/upgrading.md with a "When upgrading isn't enough — re-fork & keep your data" section (data-vs-framework mental model, upgrade-vs-re-fork decision table, data-preservation via split-portfolio + manual fallback, and the project-tickets-filed-against-the-framework-repo trap), and links docs/upgrading.md from the README. Refs #560.
Checklist Results
- ✅ Architecture & Design: N/A (docs only)
- ✅ Code Quality: Pass
- ✅ Testing: N/A (docs only)
- ✅ Security: Pass — no private project names leaked into this public-repo doc
- ✅ Performance: N/A
- ✅ PR Description & Glossary: Pass — glossary present (5 terms), summary bullets narrative
- ⚠ Summary Bullet Narrative: Pass
- ✅ Technical Decisions (AgDR):N/A — docs only, no technical decision introduced
- ✅ Adopter Handbooks: N/A — no findings (migration-safety blocking handbook concerns migration files/schema changes; this diff touches none)
Accuracy verification (the load-bearing part for a docs PR)
/updateflags described —--dry-run,--rebase,--from-version vN.N.N— all match.claude/skills/update/SKILL.md.--dry-runcorrectly described as "walk the steps without executing / no state change", matching the skill's "Does NOT execute migrations — only previews the planned chain"./split-portfolioflags —--dry-run("walk the steps without executing") and--verify("read-only state report") — both match.claude/skills/split-portfolio/SKILL.mdexactly.- The new section does NOT contradict the existing
/updatemigration-chain content above it; it complements it (the migration chain handles the upgrade-in-place path; the new section handles the re-fork path). - Re-fork guidance (copy data bucket out → re-fork → copy back → re-add
upstreamremote) is correct; thegit remote add upstreamcommand is accurate. - The "project tickets filed against the framework repo" trap and its
/updatefix line up with the per-project tracker routing + leak-protection model. - The data-vs-framework bucket table is accurate —
apexyard.projects.yaml,onboarding.yaml,projects/,workspace/,handbooks/, custom skills are correctly classified as adopter-owned data.
Mechanical checks
npx markdownlint-cli2 docs/upgrading.md README.md→ 0 errors (matches the pre-push gate).- README link target
docs/upgrading.mdresolves (file exists). - Internal link
docs/agdr/AgDR-0032-update-chain-migrations.mdin the file header is unchanged and pre-existing. - Leak scan: none of the 14 registered private project names appear in the diff.
Issues Found
None.
Suggestions
- nit (non-blocking): the trap section says upgrading "enables leak-protection so private project names don't leak into public trackers." Leak-protection is a hook that ships with the framework; "enables" is fine shorthand, but a curious reader may wonder whether it's opt-in. Optional one-clause clarification — not worth a revision.
Verdict
APPROVED
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 09db8766ab6df227fd1b1990d2f972342a040954
- Collapse two tables + a duplicate subsection into direct prose - Lead with the one idea (framework vs your data), then the few steps - Shorter, warmer, straight to the point Refs #560
atlas-apex
left a comment
There was a problem hiding this comment.
Code Review: PR #561 (re-review) — VERDICT: APPROVED
Commit: a39ff4f8053e48903d79ca7078669c134e9cfde3
(Posted as a comment — GitHub blocks self-approval on a single-account setup. Verdict is APPROVED; Rex approval marker written for the merge gate.)
Re-reviewed at current HEAD after commit a39ff4f rewrote the new section into tighter prose (net ~-35 lines, collapsing two tables + a duplicate subsection). Prior approval on the earlier commit is superseded by this one.
Summary
Docs-only change (2 files). Adds a "When /update isn't enough — re-forking & keeping your data" section to docs/upgrading.md covering the data-vs-framework mental model, an upgrade-vs-re-fork decision, split-portfolio as the durable data-preservation answer (+ manual copy-out/copy-back fallback), and the "project tickets filed against the framework repo" trap. Also links docs/upgrading.md from the README.
Checklist Results
- Architecture & Design: N/A (docs-only)
- Code Quality: Pass
- Testing: N/A (docs-only)
- Security: Pass — no private project-name leaks into this public-repo doc
- Performance: N/A
- PR Description & Glossary: Pass — Glossary present (5 terms), Summary bullets narrative
- Technical Decisions (AgDR):N/A — no technical decision introduced
- Adopter Handbooks: N/A — no handbooks triggered by a docs-only diff
Verification at HEAD (a39ff4f)
- Flag accuracy — all references match current SKILL.md behaviour:
/update --dry-run✓ ("preview only, don't touch anything")/split-portfolioone-time migration, every step prompts ✓ (confirmation-gated destructive flow)/split-portfolio --dry-run✓ ("walk through every step printing the commands that would run, but execute none")git remote add upstream https://github.com/me2resh/apexyard.git✓ correct URL/updatetracker-routing + leak-protection fix ✓ consistent with leak-protection.md
- Links — README →
docs/upgrading.mdrelative link resolves; no broken links in the new section. - markdownlint — clean under the repo's
.markdownlint.json(the relaxed ruleset CI's markdown-lint.yml actually applies; MD013/MD060 disabled there). 0 errors on the exact PR HEAD blob. - Tightened prose — no dangling references to the removed tables/subsection. The collapsed tables became the framework/data bullet list (L161-165) and the upgrade-or-re-fork prose (L167-171); both read correctly standalone.
- Leak check — scanned the new section against every registered private project name in the portfolio registry: none appear. Clean for a public-repo doc.
Issues Found
None.
Suggestions
None blocking. Reads well and concisely.
Verdict
APPROVED
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: a39ff4f8053e48903d79ca7078669c134e9cfde3
Summary
docs/upgrading.md(rather than adding a new file — avoids doc fragmentation) with a new section, "When upgrading isn't enough — re-fork & keep your data", covering the gap that let external adopters mis-file project tickets upstream from stale forks (the batch just triaged: [Chore] CSO remediations: pin CI actions, drop unused blendavit npm dep #551, [Feature] Blendavit pre-launch commerce UX (cart, entry modal, preorder flow) #556, [Feature] PDP trust rail claim chips below CTA #558, PR chore(#551): pin GitHub Actions SHAs and gitignore blendavit node_modules #552)./updatefix.docs/upgrading.mdfrom the README — it was previously only reachable fromdocs/multi-project.md.Testing
npx markdownlint-cli2 docs/upgrading.md README.md→ 0 errors (matches the pre-push gate)./updateand/split-portfolioreferences match their current SKILL.md behaviour.Refs #560
Glossary
/updatewithout re-forking.me2resh/apexyardwhen the existing fork has diverged too far to merge.