fix(migration): fix migration when another model with a different id exists

vincent-herlemont / native_db

Drop-in embedded database in Rust

MIT License

433 stars 17 forks source link

Closed croissong closed 3 months ago

croissong commented 3 months ago

fixes #159

vincent-herlemont commented 3 months ago

The CI failed due to a token issue, audit in progress here: https://github.com/vincent-herlemont/native_db/pull/161

vincent-herlemont commented 3 months ago

The CI problem is not related to the code, so it's fine by me.

@croissong You can mark the issue as ready for review if you want me to merge it.

croissong commented 3 months ago

@vincent-herlemont thanks! I added two slightly unrelated commits just now:

tried to improve the "Impossible to migrate the table ..." error message, because it confused me a bit when I encountered it, and I think it should be made clear that the issue is caused by multiple old tables with data. I hope the error is at least a little bit better now :sweat_smile:
additional tests for migrating with multiple model versions, to make sure the migrate logic still works after the change. But I just noticed that there's already test_migrate_v3 which covers multiple model versions, so let me know if I should change/remove my tests

vincent-herlemont commented 3 months ago

@croissong Thank you for improving the error message and adding the tests!

But I just noticed that there's already test_migrate_v3 which covers multiple model versions, so let me know if I should change/remove my tests

Yes, sorry for the naming of some of my tests like test_migrate_v3, which are not very descriptive. I should pay more attention to that.

This will be taken into account for Release 0.7.0. However, I am currently doing a major refactor, so it should be ready in one or two weeks.

vincent-herlemont commented 3 months ago

:tada: This PR is included in version 0.7.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: