Skip to content
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

DB migration for Crdt -> SIL.Harmony rename #1081

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

rmunn
Copy link
Contributor

@rmunn rmunn commented Sep 26, 2024

Fix #1080.

At some point the database columns used in SIL.Harmony were renamed from Crdt.Foo to SIL.Harmony.Foo, but no DB migration was created. I discovered this when creating a DB migration for another PR, so I'm splitting the Crdt -> SIL.Harmony rename into its own migration so that each migration does one single thing conceptually.

We've switched to version 8.0.8 of the EF Core NuGet packages, so we
should update the dotnet-ef command-line tool to the same version.
@rmunn rmunn requested a review from hahn-kev September 26, 2024 09:31
@rmunn rmunn self-assigned this Sep 26, 2024
@rmunn
Copy link
Contributor Author

rmunn commented Sep 26, 2024

It's possible that this belongs in the harmony repo rather than here; I'm in the middle of a different PR right now so I didn't want to divert my attention too much.

Copy link

github-actions bot commented Sep 26, 2024

C# Unit Tests

66 tests   66 ✅  5s ⏱️
11 suites   0 💤
 1 files     0 ❌

Results for commit 596c7c8.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

Nice catch, though I actually think all we need to do is update the snapshot, there's not actually any changes in the migration, so we should be able to delete the migration files and just submit the snapshot update.

@rmunn
Copy link
Contributor Author

rmunn commented Sep 27, 2024

Nice catch, though I actually think all we need to do is update the snapshot, there's not actually any changes in the migration, so we should be able to delete the migration files and just submit the snapshot update.

Huh. I didn't even notice that, I just did git add backend/LexData/Migrations/20240926092756_RenameCrdtColumnsToHarmony.*. Yep, we don't need an empty migration. Weird that the table rename didn't show up.

... Oh, I see it. The entity names changed, but the table name is still called CrdtCommits. So no DB changes were needed, hence the empty migration. Okay, will delete the migration files and make this a snapshot-only PR.

@rmunn rmunn requested a review from hahn-kev September 27, 2024 01:58
@rmunn rmunn merged commit 13d70f2 into develop Sep 27, 2024
7 of 8 checks passed
@rmunn rmunn deleted the chore/db-migration-for-harmony-rename branch September 27, 2024 03:15
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.

Add DB migration for Crdt -> SIL.Harmony rename
2 participants