Skip to content

chore: simplify migration memoize#1022

Open
ferhatelmas wants to merge 1 commit intomasterfrom
ferhat/simplify-memoize
Open

chore: simplify migration memoize#1022
ferhatelmas wants to merge 1 commit intomasterfrom
ferhat/simplify-memoize

Conversation

@ferhatelmas
Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

chore

What is the current behavior?

There is a generic memoize utility for migration loading but its tests are testing it at all.

What is the new behavior?

It's used in one place, not needed. Drop it and its tests.

Additional context

Indirectly, it drops some any usage as well.

Copilot AI review requested due to automatic review settings April 15, 2026 16:52
@ferhatelmas ferhatelmas requested a review from a team as a code owner April 15, 2026 16:52
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes an unused generic promise memoization helper from the migrations subsystem and replaces its only usage with a purpose-built cache for migration file loading.

Changes:

  • Deleted memoizePromise utility and its (non-covering) tests.
  • Implemented a directory-keyed in-module cache for loadMigrationFiles, including cache invalidation on rejection.
  • Simplified lastLocalMigrationName() logic (explicit empty-migrations handling, and clearer “freeze at” lookup).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/internal/database/migrations/memoize.ts Removed unused promise memoization helper.
src/internal/database/migrations/memoize.test.ts Removed tests that reimplemented memoization locally rather than testing the production helper.
src/internal/database/migrations/files.ts Added a focused migration-files cache and adjusted callers to use it.

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

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 15, 2026

Coverage Report for CI Build 24467924764

Coverage decreased (-0.002%) to 82.471%

Details

  • Coverage decreased (-0.002%) from the base build.
  • Patch coverage: 3 uncovered changes across 1 file (22 of 25 lines covered, 88.0%).
  • 1 coverage regression across 1 file.

Uncovered Changes

File Changed Covered %
src/internal/database/migrations/files.ts 25 22 88.0%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/internal/database/migrations/files.ts 1 79.31%

Coverage Stats

Coverage Status
Relevant Lines: 33981
Covered Lines: 30492
Line Coverage: 89.73%
Relevant Branches: 7460
Covered Branches: 3685
Branch Coverage: 49.4%
Branches in Coverage %: Yes
Coverage Strength: 349.02 hits per line

💛 - Coveralls

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas force-pushed the ferhat/simplify-memoize branch from 57dd4cd to c1317a4 Compare April 15, 2026 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants