Skip to content

fix: align sourceFileVersions parsing with maxEmbeddedVersion#2194

Open
mesutoezdil wants to merge 1 commit into
kagent-dev:mainfrom
mesutoezdil:fix/migrate-source-file-versions-parsing
Open

fix: align sourceFileVersions parsing with maxEmbeddedVersion#2194
mesutoezdil wants to merge 1 commit into
kagent-dev:mainfrom
mesutoezdil:fix/migrate-source-file-versions-parsing

Conversation

@mesutoezdil

@mesutoezdil mesutoezdil commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

sourceFileVersions used strings.SplitN and skipped files without underscore, but maxEmbeddedVersion in runner.go uses fmt.Sscanf on any leading integer. this patch makes both consistent.

Copilot AI review requested due to automatic review settings July 9, 2026 16:37
@mesutoezdil mesutoezdil requested a review from a team as a code owner July 9, 2026 16:37
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Jul 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Aligns the CLI migration filename version parsing (sourceFileVersions) with the runtime embedded-migrations parsing (maxEmbeddedVersion) by accepting any .up.sql file whose filename begins with decimal digits, rather than requiring an underscore separator.

Changes:

  • Updated sourceFileVersions to parse versions using fmt.Sscanf(name, "%d", &v) to match maxEmbeddedVersion behavior.
  • Broadens recognized migration filenames from NNN_name.up.sql to any .up.sql filename with a leading integer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread go/core/pkg/cli/db/migrate/migrate.go Outdated
Comment thread go/core/pkg/cli/db/migrate/migrate.go Outdated
@mesutoezdil mesutoezdil force-pushed the fix/migrate-source-file-versions-parsing branch from 1ccb40d to 083df7e Compare July 9, 2026 16:44
@EItanya

EItanya commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@iplay88keys

@mesutoezdil mesutoezdil force-pushed the fix/migrate-source-file-versions-parsing branch 9 times, most recently from fd959d0 to 79e171e Compare July 13, 2026 12:56
@iplay88keys

Copy link
Copy Markdown
Contributor

It would be good to align the approaches, but this is the wrong direction. If we go with this approach, something like 000007test.up.sql would be counted, but golang-migrate would silently skip that. The golang-migrate regex requires underscores, but skips any files that don't match that.

So I think the right approach would be to call golang-migrate's source.DefaultParse(name) directly to make sure that it can't drift.

@mesutoezdil mesutoezdil force-pushed the fix/migrate-source-file-versions-parsing branch from 79e171e to 2297b7e Compare July 13, 2026 19:04
Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
@mesutoezdil mesutoezdil force-pushed the fix/migrate-source-file-versions-parsing branch from 2297b7e to 106bbb0 Compare July 14, 2026 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants