| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
No issues found across 1 file
Not reviewed (too large): docs/migration.md (~2,117 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
Sorry, something went wrong.
📚 Documentation preview
|
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for fixing the two dangling whats-new.md anchors from my earlier comment — commit 426de27 resolves that, and I found no new issues this run. Still deferring to a human for a final pass, since this is a large editorial rewrite of the migration guide whose behavioral claims (which v1 examples run, what was cut as non-migration content) can't be fully verified mechanically.
What was reviewed:
This PR is documentation-only: it trims docs/migration.md from 91 to 73 sections, removing entries that were changelog/deprecation notes rather than genuine v1-to-v2 breaking changes, correcting several before/after code blocks, and folding near-duplicate entries. It also drops two links from docs/whats-new.md whose target sections the trim removes — the exact issue a prior run of this review flagged, now resolved by commit 426de27.
None. No code, configuration, or CI changes — only Markdown under docs/. The removed sections include some security-adjacent content (SEP-2243 header validation, credential binding), but removing documentation of shipped behavior does not change the behavior itself.
Docs-only changes warrant moderate scrutiny: the failure mode is misleading migration guidance, not broken software. I mechanically verified everything verifiable — all cross-page and intra-document anchors resolve, no other docs page links to a deleted section, and relative links point at real (or build-generated) files. What cannot be verified mechanically is the content accuracy of the rewritten prose and code examples: the author states every surviving before/after block was executed against v1.x and main respectively, but I cannot reproduce that here, and the judgment calls about which entries count as "genuine migration content" (e.g. dropping the SEP-2352/SEP-2350 notes and the "Notes for 2026-era connections" group entirely) are editorial decisions a maintainer should sign off on.
The prior review cycle worked as intended: the one concrete defect (dangling anchors) was reported inline and fixed in a follow-up commit before this run. The bug hunting system found nothing further. Given the size of the rewrite (~700 changed lines in migration.md) and the repository's own AGENTS.md emphasis on migration-guide accuracy for the v2 line, a human skim of the removed and rewritten sections is the remaining review work — hence deferral rather than approval.
Sorry, something went wrong.
The guide had accumulated entries that are not v1-to-v2 migration items: new v2 feature notes, "what did not change" reassurance sections, spec-adoption commentary, deprecations of APIs that still work, and internal fixes with nothing for a migrator to change. Remove those, including the whole Deprecations group; the two deprecation consequences a migrator does act on (warnings-as-errors test runs, and client calls that die on the default 2026-era connection) fold into the testing section. Correct code that was wrong for v1.x or stale for current main -- including the `mcp.types` alias now being permanent -- and verify every remaining before/after example against the v1.x and main trees. Reframe the 2026-era notes as behavior changes migrated code sees on v2's default connection, fold closely related entries together, standardize on the documented `from mcp.server import MCPServer` and `mcp.types` imports, regenerate the index and suggested-order blocks, and cut history and over-explanation throughout. Also drop two whats-new.md links whose target sections this removes (the strict docs build aborts on missing anchors). No-Verification-Needed: documentation-only change
| Back | FazBrowse Home | New Git URL |
Cuts docs/migration.md down to what it is for: someone with working v1.x code that breaks (or silently changes behavior) on v2, and needs to know what to write instead. Only that file changes.
Motivation and Context
The guide had drifted into a changelog. It carried new-in-v2 feature notes, spec-adoption commentary, deprecations of APIs that still work, and internal fixes with nothing for a migrator to change, alongside genuinely stale before/after code and a fair amount of history narration ("this was X, then Y"). A porter or an agent pointed at the file had to sieve for the parts that actually apply.
What changed:
How Has This Been Tested?
Each surviving before/after example was executed or import-checked against the corresponding tree (v1.x for "Before", main for "After"), and every intra-document anchor and relative docs link was checked. markdownlint passes.
Breaking Changes
None — documentation only.
Types of changes
Checklist
Additional context
Also touches docs/whats-new.md, but only to drop two links whose target sections this removes (the mcp dev/mcp install pinning note and the Mcp-Param-* header section) — the strict docs build aborts on missing anchors. Every other heading that another page links to was left intact.
AI Disclaimer