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

WIP: AppKeepers struct concept #696

Closed
wants to merge 2 commits into from
Closed

WIP: AppKeepers struct concept #696

wants to merge 2 commits into from

Conversation

ValarDragon
Copy link
Member

Closes: #XXX

Description

Makes a separate AppKeepers struct, to avoid issues like the v6 upgrade from occuring. We store all Keepers as pointers, and separate out hook setting from them.

Moreover, by having the keepers be a separate struct, we can have upgrades take in AppKeepers directly without circular dependencies.

The main reason for a second struct in a different module is the upgrades module. Otherwise we could keep everything in the app.go, with this new method structure & local variable separation.

Thoughts on this approach / if its worth continuing as a separate module / struct, versus just separating initialization methods in app.go?

This is going to break many tests / require aliasing, which I'm now thinking may not be worth the more convenient upgrade logic.


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@ValarDragon
Copy link
Member Author

I'd rather replace this with the approach in #697

@ValarDragon ValarDragon deleted the dev/appkeepers branch August 4, 2022 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

1 participant