Skip to content

Upgraded knex-migrator to remove sqlite3#29011

Open
sam-lord wants to merge 3 commits into
mainfrom
remove-sqlite3
Open

Upgraded knex-migrator to remove sqlite3#29011
sam-lord wants to merge 3 commits into
mainfrom
remove-sqlite3

Conversation

@sam-lord

@sam-lord sam-lord commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Completely removed the sqlite3 dependency, switching entirely to better-sqlite3 by upgrading to the new release of knex-migrator.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a3e0d375-7049-4f46-a536-9a9ed0aa0535

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

This change updates pnpm-workspace.yaml by removing sqlite3: true from allowBuilds, bumping knex-migrator from 5.4.1 to 6.0.0, removing the knex-migrator>knex override, and deleting a minimumReleaseAgeExclude comment. It also changes getNextRevisionCreatedAt in the automations repository to use moment with DATABASE_DATE_FORMAT for parsing, comparison, and formatting of revision timestamps.

Changes

File Change
pnpm-workspace.yaml Removed sqlite3: true from allowBuilds; updated knex-migrator to 6.0.0; removed 'knex-migrator>knex': 2.4.2; removed a minimumReleaseAgeExclude comment
ghost/core/core/server/services/automations/database-automations-repository.ts Switched next revision timestamp logic from Date-based parsing/comparison to moment with DATABASE_DATE_FORMAT

Possibly related PRs

Suggested labels: migration

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: upgrading knex-migrator to remove sqlite3.
Description check ✅ Passed The description is directly related to the changeset and correctly describes the sqlite3 removal.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove-sqlite3

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Jul 1, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 3004088

Command Status Duration Result
nx run @tryghost/admin-x-settings:test:acceptance ✅ Succeeded 10m View ↗
nx run ghost:test:ci:integration ✅ Succeeded 2m 42s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded <1s View ↗
nx run-many -t test:unit -p ghost,@tryghost/adm... ✅ Succeeded 6m 7s View ↗
nx run ghost:test:integration ✅ Succeeded 2m 45s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 29s View ↗
nx run ghost-admin:test ✅ Succeeded 2m 43s View ↗
nx run @tryghost/admin:build ✅ Succeeded 1m 55s View ↗
Additional runs (7) ✅ Succeeded ... View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-01 13:49:14 UTC

sam-lord added 2 commits July 1, 2026 14:01
When the lockfile was regenerated on this branch, transitive codemirror packages (@codemirror/view, @codemirror/commands) floated to versions that pull @codemirror/state@6.7.0, while admin-x-design-system still pinned 6.6.0. The two versions produced incompatible ReactCodeMirrorRef types, breaking the admin-x-settings TypeScript build (and every CI job that depends on it). Collapsing @codemirror/state back to a single 6.6.0 (matching main) resolves it, without churning any other dependencies.
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.35%. Comparing base (cd4f5fd) to head (3004088).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ces/automations/database-automations-repository.ts 54.54% 5 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #29011   +/-   ##
=======================================
  Coverage   74.34%   74.35%           
=======================================
  Files        1565     1565           
  Lines      135730   135736    +6     
  Branches    16490    16504   +14     
=======================================
+ Hits       100909   100922   +13     
- Misses      33795    33815   +20     
+ Partials     1026      999   -27     
Flag Coverage Δ
admin-tests 55.25% <ø> (-0.03%) ⬇️
e2e-tests 76.49% <54.54%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Comment thread pnpm-workspace.yaml
# Ghost's migrations build queries with the 2.4.2 query builder and knex-migrator
# executes them, so its knex must match Ghost's major or the 3.x runner throws on
# the 2.x builder. Pin it back to 2.4.2 until ghost/core itself moves to knex 3.x.
'knex-migrator>knex': 2.4.2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sam-lord I know we reverted the knex 3 dep in knex-migrator, but I'm wondering if it's worth using peer deps in knex-migrator to make the knex 3 migration a bit easier to do in Ghost directly, rather than requiring a bump in both knex-migrator and Ghost at the same time 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants