Refactor out BaseApp, StoreApp (per spec #1 and #2) #266
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
StoreApp just the queries, BaseApp with handler/ticker Ticker now defined top level, as an interface, with context Name for info taken as parameter, start cmd uses commandline name Cleaner compisition of apps.
|
Replaces #257 which i accidentally merged before review |
|
Verbally approved by Jae in SDK meeting |
ParthDesai
pushed a commit
to ChorusOne/cosmos-sdk
that referenced
this pull request
Apr 19, 2021
* Add ECOSYSTEM.md; update README.md * Spellcheck; rebuild * Add section for explorers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a big cleanup of the app subdirectory and removing all basecoin related code and naming from the app common directory. Also code cleanup and better error checking on genesis file parsing.
I changed the tick from a function to an interface and made it top-level. You can still use a function and cast it to sdk.TickerFunc to fulfill the interface. I also added a context there, so you can at least get the current height along with the datastore when processing the ticks.
There are now StoreApp which handles handshaking and queries, and BaseApp which handles DeliverTx, CheckTx, and the Tick functionality.
The genesis loading and InitState functionality was refactored out, but I still don't like the InitState method on BaseApp, but I want to tackle that as part of the bigger changes that come with proposal #3.
All code still runs, breaking changes is how to set up apps (breaking the test code) and the signature for Ticker.
One question I have is package name.
I am afraid putting it top level will lead to import cycles.
I was thinking of putting the contents of app inside server package.
But open to many suggestions
Working on the godoc, please take a look here before looking at the code, this is likely the entry point for new users.
http://localhost:6060/pkg/github.com/cosmos/cosmos-sdk/app/
http://localhost:6060/pkg/github.com/cosmos/cosmos-sdk/genesis