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

Rho: clean gaia/app #1557

Closed
5 tasks
yaruwangway opened this issue Jun 23, 2022 · 8 comments
Closed
5 tasks

Rho: clean gaia/app #1557

yaruwangway opened this issue Jun 23, 2022 · 8 comments
Assignees

Comments

@yaruwangway
Copy link
Contributor

yaruwangway commented Jun 23, 2022

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?
@Anmol1696
Copy link
Contributor

Anmol1696 commented Jun 24, 2022

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.

@yaruwangway
Copy link
Contributor Author

yaruwangway commented Jun 24, 2022

@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!

@Anmol1696
Copy link
Contributor

Anmol1696 commented Jun 25, 2022

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

@Anmol1696
Copy link
Contributor

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.

@yaruwangway
Copy link
Contributor Author

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 ?

@julienrbrt
Copy link
Member

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

@yaruwangway
Copy link
Contributor Author

yaruwangway commented Jul 8, 2022

@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 ?

@tac0turtle
Copy link
Member

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.

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

No branches or pull requests

5 participants