refactor!: kill basic manager #19512
Conversation
|
Important Auto Review SkippedMore than 25% of the files skipped due to max files limit. Skipping review to prevent low quality review. 63 files out of 170 files are above the max files limit of 100. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
| @@ -110,17 +105,7 @@ func init() { | |||
| // AppConfig returns the default app config. | |||
| func AppConfig() depinject.Config { | |||
| return depinject.Configs( | |||
| // appconfig.LoadYAML(AppConfigYAML), | |||
| appConfig, | |||
| depinject.Supply( | |||
There was a problem hiding this comment.
Now that we include that in app module directly, this is much better. The good defaults are set by depinject but can still be overwritten by optional input in to be provided, without needing to supply a whole app module basic map.
github-actions
bot
added
C:x/staking
C:CLI
C:x/gov
C:x/slashing
C:x/distribution
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for _, b := range m.Modules { | ||
| if mod, ok := b.(HasRegisterInterfaces); ok { | ||
| mod.RegisterInterfaces(registry) | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for _, b := range m.Modules { | ||
| if mod, ok := b.(HasGenesisBasics); ok { | ||
| genesisData[mod.Name()] = mod.DefaultGenesis(cdc) | ||
| } else if mod, ok := b.(HasName); ok { | ||
| genesisData[mod.Name()] = []byte("{}") | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| for _, b := range m.Modules { | ||
| // first check if the module is an adapted Core API Module | ||
| if mod, ok := b.(HasGenesisBasics); ok { | ||
| if err := mod.ValidateGenesis(cdc, txEncCfg, genesisData[mod.Name()]); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
|
TODO: update changelog + upgrading.md |
julienrbrt
changed the title
Description
Closes: #17927
Thanks to dependency injection, the need of the AppModuleBasic and the basic manager is not required anymore.
Additionally, for apps not using depinject, the basic manager was still redundant since we've allowed to create a basic manager from a module manager in previous version.
This PR simplifies the Cosmos SDK by totally removing the basic manager
Additionally, this deprecates (but not remove) the AppModuleBasic interface, and adds extension interfaces in types/module. The module manager now takes the role for the Basic Manager and implements all the necessary methods.
Furthermore, this PR removes all AppModuleBasic from all core SDK modules and put those methods directly on AppModule and implement only the relevant extension interfaces.
It updates the documentation to scrap all mention of app module basic and basic manager.
However, app dev can leave their module as-is if they wish to.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...