-
Notifications
You must be signed in to change notification settings - Fork 302
Add new string types to tables section of docs #2699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis PR updates a Markdoc documentation page for database table column types: it replaces uses of the Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/routes/docs/products/databases/tables/`+page.markdoc:
- Around line 1245-1249: Update the doc to stop showing deprecated `string` in
examples and clarify migration rules: change any example columns using `type:
'string'` to a recommended replacement such as `type: 'varchar'` (or
`text`/`mediumtext`/`longtext` as appropriate) and edit the "Migrating from
string columns" info block to state explicitly whether the migration chooses the
resulting text type based on the declared column size (e.g., the `size` option
you specify) versus the size of existing row data—if the implementation uses the
declared size, say "based on the declared size (`size` option)"; if it inspects
actual data, say "based on the maximum size of existing data"—so consumers and
examples (e.g., snippets that previously used `type: 'string'`) are consistent
with the guidance.
🧹 Nitpick comments (1)
src/routes/docs/products/databases/tables/+page.markdoc (1)
1227-1232: Clarify howvarcharvstextshould be chosen.Both rows show the same 16,383-character limit and near-identical descriptions (Line 1229–1232), which makes the selection criteria unclear. Consider adding a short note distinguishing intended usage (e.g.,
varcharrequires asize,textis for longer prose), or adjust limits if they differ.
Summary by CodeRabbit