Move fork and upgrade logic into sub-directory structure #680
Conversation
Codecov Report
@@ Coverage Diff @@
## main #680 +/- ##
==========================================
+ Coverage 18.39% 18.69% +0.30%
==========================================
Files 173 171 -2
Lines 24321 23814 -507
==========================================
- Hits 4473 4452 -21
+ Misses 19086 18600 -486
Partials 762 762
Continue to review full report at Codecov.
|
ValarDragon
changed the title
There was a problem hiding this comment.
Full support on the idea, utAck on the implementation. Looks good to me, but we should probably try to test it somehow?
I think the idea of separating out each upgrade and then eventually commenting them out once the code is no longer logically reachable makes sense.
Also interesting that the v6 upgrade has essentially no contact with this refactor, because it does no actual state migrations.
|
Good point, this should have tests before we go through it. I'll write a test for v3 and v5 upgrades. (I'll do this by checking the parameters set during the upgrade) |
|
Committed a test that updates the v4 upgrade keeper. (It reinstates an old test with mild adjustments, that got deleted in #538 ) This shows that the keeper reference setup is generally working here. Used v4 since it tests migration & param changes. The thing thats not tested is adding new stores during an upgrade, but thats really essentially the same as what exists right now |
|
Proceeding with merge, since Unity approved, and test added. (and want to try out other app.go simplifications) |
* Move fork logic into sub-directory structure * Move upgrade handlers into sub-directories * Move upgrade logic to end of keeper initialization * Add v4 migration test
Cref: #671
Description
Pull request for trying to re-arranging around the fork and migration code. This moves them, to their own sub-directory.
I mainly did this to play around with some of the code structure here. The main bottleneck was import cycles due to app.go, and having to pass around keepers instead of the app.
I suggest we keep this structure, and then make a second "AppKeepers" struct, which actually stores all of the keepers. Then OsmosisApp stores a reference to an AppKeepers struct that we pass to all of these migration logics.
I think these sub-folders for each upgrade will make it easier to track old upgrade code, and keep a reasonable location to have it be commented out, without cluttering an important file like app.go.
Thoughts?
For contributor use:
docs/) or specification (x/<module>/spec/)Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorer