docs: collect all migrations documentation in one place #11674
Conversation
|
@alumni what do you think about this one? |
WalkthroughReorganizes migration content into a dedicated "Migrations" section (11 pages), removes duplicated CLI migration instructions from other docs, updates cross-references to point to the new migration pages, and adds redirects plus a "Migrations" sidebar category. Changes
Sequence Diagram(s)(omitted — documentation-only changes; no runtime control-flow modifications) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-07-27T20:50:10.364ZApplied to files:
🔇 Additional comments (2)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
docs/docs/migrations/10-extra.md (1)
5-5: Clarify the timestamp format.Consider specifying that the timestamp should be a Unix timestamp in milliseconds for better clarity, e.g., "pass the timestamp (should be a non-negative Unix timestamp in milliseconds)".
docs/docs/migrations/02-setup.md (1)
36-36: Consider hyphenating compound adjectives for consistency.For improved readability, consider hyphenating these compound adjectives:
- Line 36: "fine grained" → "fine-grained"
- Line 47: "error prone" → "error-prone"
- Lines 49, 55: "auto run" → "auto-run"
Also applies to: 47-47, 49-49, 55-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
docs/docs/advanced-topics/3-indices.md(1 hunks)docs/docs/advanced-topics/6-using-cli.md(0 hunks)docs/docs/data-source/2-data-source-options.md(2 hunks)docs/docs/entity/5-view-entities.md(1 hunks)docs/docs/getting-started.md(2 hunks)docs/docs/help/1-faq.md(2 hunks)docs/docs/migrations/01-why.md(1 hunks)docs/docs/migrations/02-setup.md(1 hunks)docs/docs/migrations/03-creating.md(1 hunks)docs/docs/migrations/04-generating.md(1 hunks)docs/docs/migrations/05-executing.md(1 hunks)docs/docs/migrations/06-reverting.md(1 hunks)docs/docs/migrations/07-status.md(1 hunks)docs/docs/migrations/08-faking.md(1 hunks)docs/docs/migrations/09-api.md(1 hunks)docs/docs/migrations/10-extra.md(1 hunks)docs/docs/migrations/11-vite.md(1 hunks)docs/redirects.ts(1 hunks)docs/sidebars.ts(1 hunks)
💤 Files with no reviewable changes (1)
- docs/docs/advanced-topics/6-using-cli.md
🧰 Additional context used
🪛 LanguageTool
docs/docs/data-source/2-data-source-options.md
[grammar] ~66-~66: Use a hyphen to join words.
Context: ...(../migrations/01-why.md) should be auto run on every application launch. - `m...
(QB_NEW_EN_HYPHEN)
docs/docs/migrations/08-faking.md
[grammar] ~22-~22: Use a hyphen to join words.
Context: ...tion all` flag. If you require more fine grained transaction control, you can use...
(QB_NEW_EN_HYPHEN)
docs/docs/migrations/04-generating.md
[style] ~3-~3: As a shorter alternative for ‘able to’, consider using “can”.
Context: # Generating TypeORM is able to automatically generate migration files ...
(BE_ABLE_TO)
docs/docs/migrations/02-setup.md
[grammar] ~36-~36: Use a hyphen to join words.
Context: ...o specify exact classes to get more fine grained control: ```ts import FirstMigr...
(QB_NEW_EN_HYPHEN)
[grammar] ~47-~47: Use a hyphen to join words.
Context: ...quires more manual work and can be error prone. - migrationsRun - Indicates ...
(QB_NEW_EN_HYPHEN)
[grammar] ~49-~49: Use a hyphen to join words.
Context: ...(../migrations/01-why.md) should be auto run on every application launch. ## Opt...
(QB_NEW_EN_HYPHEN)
[grammar] ~55-~55: Use a hyphen to join words.
Context: ... Indicates if migrations should be auto run on every application launch. Default...
(QB_NEW_EN_HYPHEN)
[grammar] ~67-~67: Ensure spelling is correct
Context: ...ansaction mode when running migrations. Poossible options are: - all (default) - wra...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/docs/migrations/11-vite.md
[style] ~5-~5: Consider a more concise word here.
Context: ...nify) which includes mangling your code in order to minimize file sizes. You have 3 option...
(IN_ORDER_TO_PREMIUM)
[grammar] ~7-~7: Ensure spelling is correct
Context: ... mitigate this. The 3 options are shown belown as diff to this basic vite.config.ts ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~65-~65: Use a hyphen to join words.
Context: ...lower then esbuild, but offers more fine grained control over what to minify. Add...
(QB_NEW_EN_HYPHEN)
docs/docs/help/1-faq.md
[style] ~213-~213: Consider a more concise word here.
Context: ...fault which includes mangling your code in order to minimize file sizes. This breaks the [m...
(IN_ORDER_TO_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
docs/docs/migrations/05-executing.md
9-9: Link fragments should be valid
(MD051, link-fragments)
docs/docs/migrations/11-vite.md
29-29: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (16)
docs/sidebars.ts (1)
38-42: LGTM!The new Migrations category is well-placed in the sidebar hierarchy and follows the established pattern of using autogenerated items for documentation directories.
docs/docs/entity/5-view-entities.md (1)
14-14: LGTM!The link to the migrations documentation is correctly formatted and uses the appropriate relative path.
docs/docs/advanced-topics/3-indices.md (1)
151-151: LGTM!The migrations documentation link is correctly placed in context and properly formatted.
docs/docs/migrations/06-reverting.md (1)
1-11: LGTM!The documentation clearly explains the reverting process. The double dash (
--) before the-dflag is correct for passing arguments through npm/yarn scripts to the underlying TypeORM CLI.docs/docs/migrations/07-status.md (1)
1-11: LGTM!The status documentation is clear and concise. The legend explaining the
[X]and[ ]markers is particularly helpful for users.docs/docs/getting-started.md (2)
37-37: LGTM!The migration feature now properly links to the dedicated migrations guide, making it more discoverable for users.
1198-1198: LGTM!The extension description now includes a helpful link to the migrations documentation.
docs/docs/migrations/09-api.md (2)
1-97: LGTM!The comprehensive migration example effectively demonstrates QueryRunner API usage with practical operations like creating tables, indexes, and foreign keys. The corresponding
downmethod properly reverses all operations.
99-630: LGTM!The API reference is thorough and well-organized. Each method includes clear parameter descriptions, return types, and relevant database-specific notes (e.g., MySQL vs PostgreSQL differences).
docs/docs/migrations/10-extra.md (1)
8-8: ****The command syntax in the documentation is correct. The official TypeORM documentation confirms that both
typeorm -t 1650000000000 migration:createandtypeorm migration:generate -t 1650000000000are valid, meaning the flag can be positioned either before or after the subcommand. The syntax in the file matches the documented example and requires no changes.Likely an incorrect or invalid review comment.
docs/redirects.ts (1)
126-126: LGTM!The redirect properly supports the migration documentation consolidation by redirecting the old path to the new location.
docs/docs/migrations/08-faking.md (1)
26-42: LGTM!The example correctly demonstrates how to use
transaction = falsewith PostgreSQL'sCONCURRENTLYoperations, which cannot run inside a transaction block.docs/docs/migrations/03-creating.md (1)
40-56: LGTM!The SQL syntax correctly uses PostgreSQL's
RENAME COLUMNsyntax. The example properly demonstrates both theup(rename title to name) anddown(revert back) methods.docs/docs/migrations/11-vite.md (1)
9-86: LGTM!The three mitigation options are well-explained with clear configuration examples. The diff format makes it easy to understand what changes to make. Option 3 (terser) provides the best balance between file size and functionality by specifically preserving only migration class names.
docs/docs/data-source/2-data-source-options.md (1)
64-70: Consolidated migration option descriptions appropriately reference new migrations guide.The refactoring replaces detailed descriptions of migration-related options with concise links to the centralized migrations documentation at
../migrations/01-why.md. The descriptions are now uniform and consistent across all four options (migrations,migrationsRun,migrationsTableName,migrationsTransactionMode).Verify that the target documentation (
../migrations/01-why.md) provides sufficient context for users to understand each option, especially since detailed descriptions are removed here.docs/docs/help/1-faq.md (1)
183-211: Webpack configuration example is comprehensive and well-structured.The expanded Webpack entry configuration (lines 187–210) is significantly improved. It now clearly demonstrates:
- Dynamic glob-based entry generation for migration files
- Explicit
resolve.extensionsconfiguration for TypeScript- Correct UMD
libraryTargetfor migration compatibility- Proper output path separation for transpiled migrations
The dual-path configuration in the data source example (lines 250–255) is an excellent best practice, showing both production (transpiled) and development (source) paths.
Verify that the UMD library target and naming strategy (lines 206–208) align with TypeORM's current migration execution model. Specifically, confirm that TypeORM migration detection and execution still works correctly with UMD-bundled files.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…tions # Conflicts: # docs/docs/advanced-topics/3-indices.md
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/docs/migrations/11-vite.md (1)
65-65: Use hyphenation for compound adjective."Fine grained" should be "fine-grained" when modifying "control".
Apply this diff:
-Vite supports using terser as minifier. Terser is slower then esbuild, but offers more fine grained control over what to minify. +Vite supports using terser as minifier. Terser is slower then esbuild, but offers more fine-grained control over what to minify.docs/docs/migrations/02-setup.md (1)
36-36: Use hyphens for compound adjectives.Two compound adjectives should be hyphenated:
- Line 36: "fine grained" → "fine-grained"
- Line 47: "error prone" → "error-prone"
Apply this diff:
-Alternatively you could also specify exact classes to get more fine grained control: +Alternatively you could also specify exact classes to get more fine-grained control:-but it also requires more manual work and can be error prone. +but it also requires more manual work and can be error-prone.Also applies to: 47-47
docs/docusaurus.config.ts (1)
201-227: Well-structured plugin configuration with proper type safety.The expanded configuration is comprehensive and properly typed. The multiple content selectors provide good fallback coverage for different page layouts, and the strict error handling (
onRouteError: "throw") will help catch routing issues early during build.Optional: Consider verifying whether all 7 content selectors are necessary by testing which ones are actually matching content in your documentation pages. This could simplify the configuration if some selectors are redundant.
#!/bin/bash # Search for the CSS selectors in the generated HTML to understand which ones are present echo "Checking documentation about contentSelectors usage in the plugin..."@signalwire/docusaurus-plugin-llms-txt content selectors best practices
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
.github/workflows/close-stale-issues.yml(0 hunks).github/workflows/codeql.yml(1 hunks).github/workflows/commit-validation.yml(2 hunks).github/workflows/docsearch.yml(1 hunks).github/workflows/preview.yml(1 hunks).github/workflows/publish-package.yml(1 hunks).github/workflows/tests-linux.yml(11 hunks).github/workflows/tests-windows.yml(3 hunks)CONTRIBUTING.md(1 hunks)docker-compose.yml(1 hunks)docs/.gitignore(1 hunks)docs/docs/advanced-topics/3-indices.md(3 hunks)docs/docs/advanced-topics/4-listeners-and-subscribers.md(3 hunks)docs/docs/guides/1-active-record-data-mapper.md(0 hunks)docs/docs/help/3-decorator-reference.md(3 hunks)docs/docs/migrations/02-setup.md(1 hunks)docs/docs/migrations/05-executing.md(1 hunks)docs/docs/migrations/09-api.md(5 hunks)docs/docs/migrations/11-vite.md(1 hunks)docs/docs/query-runner.md(2 hunks)docs/docusaurus.config.ts(2 hunks)docs/src/pages/markdown-page.md(0 hunks)eslint.config.mjs(1 hunks)gulpfile.ts(2 hunks)package.json(3 hunks)src/commands/InitCommand.ts(3 hunks)src/driver/mongodb/MongoConnectionOptions.ts(1 hunks)src/driver/mongodb/MongoDriver.ts(1 hunks)src/driver/mongodb/typings.ts(1 hunks)src/driver/postgres/PostgresDriver.ts(1 hunks)test/functional/driver/MongoDriver.ts(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (3)
- docs/docs/guides/1-active-record-data-mapper.md
- .github/workflows/close-stale-issues.yml
- docs/src/pages/markdown-page.md
✅ Files skipped from review due to trivial changes (7)
- docs/.gitignore
- docs/docs/advanced-topics/4-listeners-and-subscribers.md
- docs/docs/query-runner.md
- docs/docs/help/3-decorator-reference.md
- src/driver/postgres/PostgresDriver.ts
- src/driver/mongodb/typings.ts
- docker-compose.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/docs/advanced-topics/3-indices.md
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-27T17:29:18.547Z
Learnt from: alumni
Repo: typeorm/typeorm PR: 11581
File: docs/docs/drivers/mongodb.md:9-13
Timestamp: 2025-07-27T17:29:18.547Z
Learning: TypeORM is compatible only with mongodb@^6, not v4 as previously suggested. The package.json file contains the authoritative peer dependency information for the MongoDB driver version compatibility.
Applied to files:
src/commands/InitCommand.tspackage.jsondocs/docs/migrations/11-vite.md
📚 Learning: 2025-07-27T17:29:18.547Z
Learnt from: alumni
Repo: typeorm/typeorm PR: 11581
File: docs/docs/drivers/mongodb.md:9-13
Timestamp: 2025-07-27T17:29:18.547Z
Learning: TypeORM supports MongoDB driver versions "^5.8.0 || ^6.0.0" according to the peerDependencies in package.json. The v4 driver is NOT supported. Always check package.json for accurate peer dependency information.
Applied to files:
src/commands/InitCommand.ts
📚 Learning: 2025-08-16T18:51:14.170Z
Learnt from: alumni
Repo: typeorm/typeorm PR: 11576
File: src/data-source/DataSource.ts:542-546
Timestamp: 2025-08-16T18:51:14.170Z
Learning: When improving type safety in TypeORM's query execution chain, adding generics to the QueryRunner.query method allows for proper type propagation from DataSource.query<T> through to QueryResult<T>, eliminating the need for type casts like 'as any' and 'as T'.
Applied to files:
docs/docs/migrations/09-api.md
🧬 Code graph analysis (1)
test/functional/driver/MongoDriver.ts (1)
src/driver/mongodb/MongoDriver.ts (2)
MongoDriver(32-575)connect(246-258)
🪛 LanguageTool
docs/docs/migrations/02-setup.md
[grammar] ~36-~36: Use a hyphen to join words.
Context: ...o specify exact classes to get more fine grained control: ```ts import FirstMigr...
(QB_NEW_EN_HYPHEN)
[grammar] ~47-~47: Use a hyphen to join words.
Context: ...quires more manual work and can be error prone. - migrationsRun - Indicates ...
(QB_NEW_EN_HYPHEN)
docs/docs/migrations/11-vite.md
[style] ~5-~5: Consider a more concise word here.
Context: ...nify) which includes mangling your code in order to minimize file sizes. You have 3 option...
(IN_ORDER_TO_PREMIUM)
[grammar] ~65-~65: Use a hyphen to join words.
Context: ...lower then esbuild, but offers more fine grained control over what to minify. Add...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: tests-linux (20) / postgres (14)
- GitHub Check: tests-linux (20) / mysql_mariadb
- GitHub Check: tests-linux (20) / postgres (17)
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (18) / sap
- GitHub Check: tests-linux (18) / postgres (14)
- GitHub Check: tests-linux (18) / postgres (17)
- GitHub Check: tests-linux (20) / mssql
- GitHub Check: tests-linux (20) / cockroachdb
- GitHub Check: tests-linux (20) / mongodb
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-linux (18) / sqljs
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (18) / mysql_mariadb_latest
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-windows / sqljs
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-windows / sqlite
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (22)
docs/docusaurus.config.ts (1)
5-5: Good addition of type safety.The import of
LLMsTXTPluginOptionsand its use with thesatisfiesoperator on line 226 provides proper type checking for the plugin configuration..github/workflows/docsearch.yml (1)
14-14: LGTM: Workflow action update.The update from
actions/checkout@v4tov5is appropriate and aligns with the v5 updates across other workflows in this PR..github/workflows/preview.yml (1)
18-18: LGTM: Workflow actions updated to v5.The updates to
actions/checkout@v5andactions/setup-node@v5are appropriate and consistent with other workflow updates in this PR.Also applies to: 23-23
.github/workflows/tests-windows.yml (1)
14-15: LGTM: Consistent workflow action updates across all jobs.All three Windows test jobs (better-sqlite3, sqlite, sqljs) have been consistently updated to use
actions/checkout@v5andactions/setup-node@v5.Also applies to: 37-38, 60-61
eslint.config.mjs (1)
86-94: LGTM: Well-documented temporary rule override.The addition of a temporary
jsdoc/valid-typeserror rule is clearly documented with an explanation of its purpose and planned removal once all warnings are addressed..github/workflows/codeql.yml (1)
26-26: LGTM: Workflow action update.The update to
actions/checkout@v5is appropriate and consistent with other workflow updates in this PR..github/workflows/commit-validation.yml (1)
21-22: LGTM: Consistent workflow action updates.Both the formatting and build jobs have been consistently updated to use
actions/checkout@v5andactions/setup-node@v5.Also applies to: 34-35
CONTRIBUTING.md (1)
21-21: LGTM: Professional security contact update.The update to use the official maintainers email (
maintainers@typeorm.io) makes the security reporting process clearer and more professional. This aligns well with the package.json author email update.package.json (3)
16-17: LGTM: Organizational branding update.The author change from a personal name to "TypeORM" with the organizational email
maintainers@typeorm.ioappropriately reflects the project's collective maintenance and aligns with the security contact update in CONTRIBUTING.md.
249-254: LGTM: Package manager enforcement added.The
devEnginesconfiguration enforces npm as the package manager withonFail: "error". This provides clear governance for contributors and prevents inconsistencies from using different package managers.
116-116: LGTM: Dev dependency enables testing for optional driver.Adding
@google-cloud/spanneras a devDependency (while it's already an optional peer dependency) enables CI testing and development without requiring manual installation. The version^7.21.0is within the declared peer dependency range^5.18.0 || ^6.0.0 || ^7.0.0..github/workflows/tests-linux.yml (2)
24-25: LGTM: GitHub Actions version bumps.The upgrade from v4 to v5 for both
actions/checkoutandactions/setup-nodeacross all jobs is a routine maintenance update and should be backward compatible.Also applies to: 51-52, 81-82, 122-123, 163-164, 184-185, 205-206, 226-227, 266-267, 287-291, 314-318
46-46: Verify compatibility with MongoDB 8.The MongoDB service image has been upgraded from
mongo:5.0.31tomongo:8, which is a major version jump. While this aligns with the docker-compose template changes in InitCommand.ts, please ensure that all tests pass successfully with MongoDB 8.Based on learnings, TypeORM supports MongoDB driver versions "^5.8.0 || ^6.0.0". Note that the MongoDB driver version and MongoDB server version are separate concerns.
tsconfig.json (1)
17-17: LGTM: Enables JSON module imports.Adding
resolveJsonModule: trueis necessary to support importing JSON files (like package.json) as modules in TypeScript, which is used in InitCommand.ts..github/workflows/publish-package.yml (1)
14-15: LGTM: Consistent GitHub Actions version updates.The upgrade to v5 for both actions is consistent with the changes in tests-linux.yml and represents routine maintenance.
test/functional/driver/MongoDriver.ts (1)
62-98: LGTM: Well-structured test for proxy options.The new test suite properly verifies that MongoDB proxy configuration options are forwarded to
MongoClient.connect. The test uses appropriate stubbing with sinon and validates all four proxy-related fields.src/driver/mongodb/MongoDriver.ts (1)
206-210: LGTM: Proxy options properly registered.The four Socks5 proxy configuration options are correctly added to
validOptionNames, which ensures they will be forwarded to the MongoDB driver viabuildConnectionOptions. This aligns with the interface changes in MongoConnectionOptions.ts and is covered by the new test.gulpfile.ts (2)
229-237: LGTM: Build-time path rewriting for package.json import.This task correctly adjusts the package.json import path in the compiled InitCommand.js during the packaging process. Since InitCommand.ts imports from
../../package.jsonand files are moved during packaging, this rewrite ensures the reference remains valid in the final build output.
256-257: Proper integration into packaging sequence.The new task is correctly integrated into the packaging sequence alongside other post-compilation cleanup tasks.
src/commands/InitCommand.ts (2)
9-9: LGTM: Dynamic dependency sourcing from package.json.Importing package.json as a module enables dynamic resolution of dependency versions throughout the init command, eliminating hardcoded versions and ensuring consistency with the project's actual dependencies.
628-628: MongoDB image version simplified.The docker-compose template now uses
mongo:8instead of a specific patch version, which aligns with the CI workflow changes and provides automatic minor/patch updates.src/driver/mongodb/MongoConnectionOptions.ts (1)
306-325: LGTM: Well-documented proxy configuration options.The four Socks5 proxy configuration properties are properly typed, documented, and follow the existing pattern in the interface. They correctly extend the public API surface for MongoDB connection options.
gioboa
left a comment
There was a problem hiding this comment.
It looks a good refactoring to me 👏
Thanks @pkuczynski
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/redirects.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: alumni
Repo: typeorm/typeorm PR: 11581
File: docs/docs/drivers/mongodb.md:9-13
Timestamp: 2025-07-27T17:29:18.547Z
Learning: TypeORM is compatible only with mongodb@^6, not v4 as previously suggested. The package.json file contains the authoritative peer dependency information for the MongoDB driver version compatibility.
Learnt from: alumni
Repo: typeorm/typeorm PR: 11581
File: docs/docs/drivers/mongodb.md:9-13
Timestamp: 2025-07-27T17:29:18.547Z
Learning: TypeORM supports MongoDB driver versions "^5.8.0 || ^6.0.0" according to the peerDependencies in package.json. The v4 driver is NOT supported. Always check package.json for accurate peer dependency information.
mguida22
left a comment
There was a problem hiding this comment.
I like the idea here of bringing together all of our migration docs into a single top level section.
I have a few comments I'd like to discuss before we make this change
- I find it very helpful when reading docs to be able to see most info on the same page so I can search and see things together. There's a few pages under 'migrations' that only have a few lines, let's see if we can organize that info together.
- There's a lot of helpful information on non-migration pages that this PR removes. We should keep that info in addition to linking to the migrations section of docs.
- There is some editing for grammar that should be done before merging. I'm happy to help with editing if you want.
| ## Create a new migration | ||
|
|
||
| You can create a new migration using CLI: | ||
|
|
||
| ```shell | ||
| typeorm migration:create path-to-migrations-dir/migrationName | ||
| ``` | ||
|
|
||
| Learn more about [Migrations](./1-migrations.md). |
There was a problem hiding this comment.
this type of information is still useful under the cli docs.
There was a problem hiding this comment.
I don't think it is. One should learn and understand how migrations works from migrations doc. If anything this page should only list all cli commands and reference to other sections in the docs (which was my longer term plan...). I just restored it in that form, please have a look.
Worth to note, that current CLI docs are outdated/not complete and the information how to run things via cli is spread across the whole docs...
There was a problem hiding this comment.
If anything this page should only list all cli commands and reference to other sections in the docs (which was my longer term plan...)
That sounds good like a good plan, but this removed the commands from the cli docs today.
There was a problem hiding this comment.
Did it? I still see them. Check above your comment or in this commit: 4d40123
| It accepts both migration classes and directories from which to load. | ||
| Directories support glob patterns. | ||
| Example: `migrations: [FirstMigration, SecondMigration, "migration/*.js", "modules/**/migration/*.js"]`. | ||
| Learn more about [Migrations](../advanced-topics/1-migrations.md). |
There was a problem hiding this comment.
we should keep this info in these docs, even if we duplicate some of it to the migration docs
There was a problem hiding this comment.
Why would we? Similarlity to CLI docs, I think this page should only be an index linking to more detailed pages...
I applied SRP principle, as I typically do in programming. Long functions, classes or files are hard to read and manage. For searching we have typesens on the top, which would actually work even better when docs are split. We also have <> buttons at the bottom. And current split with left and right navigation menu is quite confusing and bad UX. We are all a little bit different I guess and I prefer a clear split on the topics, instead of having one lengthy document...
I applied DRY principle like one would do when programming. Duplicated content is hard to manage. I moved everything into migrations section and link there from other places. I assumed that pages like CLI docs should be only a reference to more detailed sections.
I totally agree. Current docs have often poor grammar and unclear content. My goal with this PR was only to propose how documentation could be shaped in the future (merge topics together, no repeatation, split into smaller chunks, remove "advanced topics" section, etc) as a first step and later on work on the actual content. I expected that current PR might raise some controversy, so I didn't wanted to invest into the content just yet to avoid unnecessary work. I would propose to merge current PR as it is and raise another one with improvements in the content. Any help with that is of course very welcome! |
sgarner
left a comment
There was a problem hiding this comment.
Couple of tiny things but otherwise this LGTM. Thanks for your effort on this @pkuczynski 💜
PS: I thought we had a way to preview docs changes, but I can't see anything being generated for this PR?
|
@pkuczynski the vite docs don't belong in migration as they are not migration specific. |
This specific page explains how to fix migerations with Vite. Could be possibly moved somewhere else, but where? |
👍 I have a slightly different preference but that makes sense to me and I can follow that pattern going forward.
Removing most duplication is ok, but it's odd that I can no longer see these commands on the CLI page. They are still CLI commands so in my opinion we should list them on the CLI page and link to migration docs for more info on using them. There's some maintenance burden to duplicating info, but sometimes that is worth it to make it easier for users to find what they are looking for.
👍
In general I agree that smaller PRs are better, but we should not make changes that are incomplete or low quality. This PR introduced errors of it's own. It's also possible to stack PRs if we want to review changes independently.
Happy to help 😃 |
* docs: collect all migrations documentation in one place * links * vite * fic * link to vite project * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * ai review suggestions * docs: collect all migrations documentation in one place * links * vite * fic * link to vite project * ai review suggestions * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * format * single redirect * fix bad merge * auto-run * restore cli * add migration create * fix postgresql syntax --------- Co-authored-by: Giorgio Boa <35845425+gioboa@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* docs: collect all migrations documentation in one place * links * vite * fic * link to vite project * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * ai review suggestions * docs: collect all migrations documentation in one place * links * vite * fic * link to vite project * ai review suggestions * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * format * single redirect * fix bad merge * auto-run * restore cli * add migration create * fix postgresql syntax --------- Co-authored-by: Giorgio Boa <35845425+gioboa@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Description of change
I collected all documentation around migrations into a single guide (folder). Tried to do only minimalistic changes to the content. I would be happy to extend documentation around migrations in a separate step (I have few ideas)
I also linked (most) of the
migration(s)references from other documents into this guide.Fixes #11673
Pull-Request Checklist
masterbranchFixes #00000Summary by CodeRabbit