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

Move module.go methods deprecated by RegisterServices into an extension interface #11863

Closed
4 tasks
ValarDragon opened this issue May 3, 2022 · 4 comments · Fixed by #12725
Closed
4 tasks
Assignees

Comments

@ValarDragon
Copy link
Contributor

ValarDragon commented May 3, 2022

Summary

Currently, all modules in Cosmos have to pay the complexity of legacy code, by defining functions in the module.go that only made sense with legacy API's. (Adding to the overall boilerplate of the codebase)

I propose we move Route(), QuerierRoute(), LegacyQueryHandler() into their own extension interface for the module.go. Perhaps:

type LegacyRegisterServices interface {
  AppModule
  Route()
  QuerierRoute()
  LegacyQueryHandler()
}

Then any legacy code that uses this, can attempt to run-time typecast the module to get these routes / handlers. Similarly, we can then delete these 3 methods off of every legacy module's module.go. (Thereby eliminating more boilerplate from every module)

This should be a non-breaking change for module developers, unless they do things off of the Module interface (and this legacy code), which I don't expect is true.

If folks agree with this, is this something that could get into a point release? If so, it would then enable us to start deleting these lines from tooling / existing chain codebases once they update to the relevant SDK version.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

TBH we've been removing the legacy APIs from AppModule, there is no need to further support or tweak legacy code that is meant to be outright removed. We already removed RegisterRESTRoutes (See: #11797)

@ValarDragon
Copy link
Contributor Author

Gotcha. Its unclear to me what the API stability guarantees of these methods are, if they can just be deleted then happy about that!

@tac0turtle
Copy link
Member

should we sneak this into 0.46?

@alexanderbez
Copy link
Contributor

Gotcha. Its unclear to me what the API stability guarantees of these methods are, if they can just be deleted then happy about that!

Yes, the idea is that they're just deleted. In fact, we should delete whatever else is deprecated/legacy. @facundomedica is that something you're interested in? @ValarDragon pointed out the methods in the original issue body.

@tac0turtle tac0turtle self-assigned this May 3, 2022
@tac0turtle tac0turtle moved this to 📝 Todo in Cosmos-SDK May 9, 2022
Repository owner moved this from 📝 Todo to 👏 Done in Cosmos-SDK Jul 28, 2022
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 a pull request may close this issue.

3 participants