gaia icon indicating copy to clipboard operation
gaia copied to clipboard

Published 20 hours ago • verified publisher icon cosmos

Rho: clean gaia/app

Open yaruwangway opened this issue 3 years ago • 8 comments
trafficstars

Summary

clean up gaia/app/app.go after rho integration finishes.

Problem Definition

app/app.go is too big file.

Proposal

split app/app.go into app.go, upgrade/, keepers/ , module.go

app_test.go into apptest/app_test.go

ref: osmosis/app provenance/app

For Admin Use

  • [ ] Not duplicate issue
  • [ ] Appropriate labels applied
  • [ ] Appropriate contributors tagged
  • [ ] Contributor assigned/self-assigned
  • [ ] Is a spike necessary to map out how the issue should be approached?

yaruwangway avatar Jun 23 '22 09:06 yaruwangway

I would want to see this happen too. Current way of handling upgrades is very adhoc and the change in code is not limited to a clean module. Osmosis way of handling upgrades specially is something which is way cleaner and also sort of containerised.

But this would go hand in hand in splitting the whole up keepers as well as modules. Would be willing to take this up.

Anmol1696 avatar Jun 24 '22 06:06 Anmol1696

@Anmol1696 you are welcomed to take this up! 👍 but please pay attention that we are having a few prs open to work towards rho upgrade. so we will not merge this refactoring pr till other rho prs are merged.

so you can do this refactor pr early and might need to solve a lot conflicts later. or I can @ you when the major PR for rho is done and then you do this refactor. both options are fine as you like. Thank you!

yaruwangway avatar Jun 24 '22 16:06 yaruwangway

Sounds good. Thanks for the heads up. I dont mind doing conflict resolutions till this upgrade as well.

Anmol1696 avatar Jun 25 '22 06:06 Anmol1696

Any plans for use the new app rewiring and depinject module? I guess the handling of upgrades in a cleaner way should still be possible with the depinject module, when we do decide to use it here in gaia.

Anmol1696 avatar Jul 06 '22 05:07 Anmol1696

hi, @Anmol1696 , yeah, plan to use depinject module, but not sure if this will be in sdk0.46 since 0.46-rc2 does not have this yet.

Hi, @marbar3778, will depinject module in sdk 0.46 or 0.47 ?

yaruwangway avatar Jul 07 '22 11:07 yaruwangway

Hey, no, depinject won't be included in v0.46.

julienrbrt avatar Jul 07 '22 13:07 julienrbrt

@Anmol1696 , I know we will use depinject, just not sure this this will be in rho or next, next upgrade.

@okwme can you confirm here ?

yaruwangway avatar Jul 08 '22 15:07 yaruwangway

it will most likely be included in the next upgrade, post 0.46. But you will be able to write apps the current way going forward as well.

tac0turtle avatar Jul 09 '22 11:07 tac0turtle