docs: document D1 migration numbering convention - #407
Conversation
Documents the four duplicated numeric prefixes in packages/worker/migrations/ (0009, 0010, 0018, 0023), assesses the risk of doing nothing vs renaming on disk vs squashing into a baseline, and recommends a forward-only duplicate-prefix lint guardrail plus a small addition to the migration-authoring section of setup.md. Enumerates the test files (legacy-inline-sources, jobs-codemode-only, unified-email-receipt) that hardcode migration filenames so the impact of any future rename is concrete. No source or migration changes. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
Warning Rate limit exceeded
Youβve run out of usage credits. Purchase more in the billing tab. β How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. π¦ How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. βΉοΈ Review infoβοΈ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: π Files selected for processing (1)
π WalkthroughWalkthroughThis pull request consolidates D1 migration authoring guidance by introducing a dedicated "Authoring D1 migrations" documentation section with step-by-step naming conventions, conflict resolution procedures, and constraints on editing deployed migrations, while removing duplicate guidance from the "Documentation maintenance" section. ChangesD1 Migration Documentation
Estimated code review effortπ― 1 (Trivial) | β±οΈ ~3 minutes Possibly related PRs
Poem
π₯ Pre-merge checks | β 5β Passed checks (5 passed)
βοΈ Tip: You can configure your own custom pre-merge checks in the settings. β¨ Finishing Touchesπ§ͺ Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Replaces the earlier cleanup RFC with a small "Authoring D1 migrations" section in docs/contributing/setup.md. Captures: - Pick the next-highest 4-digit prefix; rebase and renumber on collision. - Existing migration files that have landed in main are immutable (kept from the previous bullet under Documentation maintenance). - The four current duplicate prefixes (0009, 0010, 0018, 0023) are grandfathered: do not rename them, and do not add a third file to any of those prefixes. The cleanup-rfcs/ scratch file is removed since we are not making any code/migration changes. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
π Preview deployed: https://kody-pr-407.kentcdodds.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
π§Ή Nitpick comments (1)
docs/contributing/setup.md (1)
113-114: β‘ Quick winConsider clarifying the duplicate-prefix prohibition.
The guidance "Do not commit a duplicate prefix" is clear, but since lines 119-124 document four existing duplicate prefixes, you might want to make the rule more precise to avoid confusion.
π Suggested clarification
- If your branch is behind `main` and a new migration has landed upstream with the prefix you picked, rebase and renumber. Do not commit a duplicate prefix. + the prefix you picked, rebase and renumber. Do not commit a duplicate prefix + (beyond the four grandfathered pairs documented below).Alternatively, you could rephrase to:
- If your branch is behind `main` and a new migration has landed upstream with - the prefix you picked, rebase and renumber. Do not commit a duplicate prefix. + the prefix you picked, rebase and renumber to avoid adding new duplicate prefixes.π€ Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/contributing/setup.md` around lines 113 - 114, Clarify the rule around duplicate migration prefixes by replacing the terse sentence "Do not commit a duplicate prefix" with an explicit instruction: state that if a migration prefix already exists upstream (see the example duplicate prefixes listed nearby), you must rebase onto main and renumber your migration to a unique unused prefix before committing, and never reuse an existing prefix even if your local file name matches β include a brief example of the before/after renumbering to illustrate; update the sentence that currently reads "Do not commit a duplicate prefix" to this more precise guidance so readers know to both rebase and choose a new unique prefix.
π€ Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/contributing/setup.md`:
- Around line 113-114: Clarify the rule around duplicate migration prefixes by
replacing the terse sentence "Do not commit a duplicate prefix" with an explicit
instruction: state that if a migration prefix already exists upstream (see the
example duplicate prefixes listed nearby), you must rebase onto main and
renumber your migration to a unique unused prefix before committing, and never
reuse an existing prefix even if your local file name matches β include a brief
example of the before/after renumbering to illustrate; update the sentence that
currently reads "Do not commit a duplicate prefix" to this more precise guidance
so readers know to both rebase and choose a new unique prefix.
βΉοΈ Review info
βοΈ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d9d7291e-3468-410d-92ff-862670eb3b4d
π Files selected for processing (1)
docs/contributing/setup.md
Address CodeRabbit nitpick on PR #407: the previous wording "Do not commit a duplicate prefix" could be read as conflicting with the following bullet that documents four grandfathered duplicate-prefix pairs. Tighten the rebase-and-renumber instruction and explicitly note that the existing pairs are exceptions, not a precedent. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Summary
packages/worker/migrations/has four duplicated 4-digit prefixes fromearlier parallel-branch merges:
0009-secret-allowed-hosts.sql/0009-ui-artifact-parameters.sql0010-secret-allowed-capabilities.sql/0010-value-buckets.sql0018-jobs.sql/0018-mcp-memory-source-uris.sql0023-entity-sources.sql/0023-secret-allowed-packages.sqlWe're not changing anything on disk: D1 tracks applied migrations by
exact filename, the apply order is stable (Wrangler sorts the full
filename), and all four pairs are additive (new tables/indexes only) so
ordering is not behaviorally significant.
This PR just documents the situation and the numbering convention going
forward, in a new "Authoring D1 migrations" section in
docs/contributing/setup.md. Specifically:mainare immutable (thisbullet was previously under "Documentation maintenance" and is moved
here since it is really a migration-authoring rule).
don't add a third file to any of those prefixes.
What this PR does not do
.sqlfile.legacy-inline-sources,jobs-codemode-only, andunified-email-receiptmigration testscontinue to reference filenames like
0009-ui-artifact-parameters.sql,0018-jobs.sql, and0023-entity-sources.sqldirectly).Verification
Docs-only change. Verified
oxfmt --checkis clean on the modified file.Summary by CodeRabbit