-
Notifications
You must be signed in to change notification settings - Fork 204
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
fix: missing migration script and exports #1184
fix: missing migration script and exports #1184
Conversation
Signed-off-by: Ariel Gentile <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1184 +/- ##
=======================================
Coverage 88.81% 88.82%
=======================================
Files 706 707 +1
Lines 16583 16593 +10
Branches 2801 2804 +3
=======================================
+ Hits 14729 14739 +10
Misses 1744 1744
Partials 110 110
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dangit. We may need to add it as a separate upgrade (like 0.3-patch.1) as agents that have already upgraded won't be able to upgrade again. Thoughts?
And time to add some tests for this!! Will add soon |
Well, at the first moment I had the problem that I was on 0.3.0.alpha.xx and when upgrading to 0.3.0 "release" it already thought that I had the last storage version while of course it wasn't. I think it could be a bit complicated to get version granularity up to alpha level, but maybe it could be possible to do it up to patch version. This way we could add this |
Signed-off-by: Ariel Gentile <[email protected]>
I've added a test to 0.2 migration considering it will go straight from 0.2.x to 0.3.1, and also a 0.3 migration test that expects to update only DIDs. Working fine also in a 'real-world' 0.3.0 wallet so I hope this will fix the problem as long as most users skip 0.3.0 when upgrading their wallets. What do you think on supporting patch versions in migrations? Theoretically we'll not need it and we will go now from storage version 0.3.1 to -> 0.4 directly, but if something unfortunate happens (as this issue) we can cover it easily. |
Signed-off-by: Ariel Gentile <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thanks @genaris. I think patch version migrations should be avoided as it makes updates harder, but yeah totally agree that in such cases it's good to have patch updates
Unfortunately,
migrateDidRecordToV0_3
was not added toupdateV0_2ToV0_3
script, meaning that DID records were not upgraded, making DIDs non-resolvable.This PR fixes that and also exposes a few classes that are mentioned in the documentation but were no exported directly.