Cosmos-sdk: Remove alias.go usage

Created on 2 Jun 2020  ·  9Comments  ·  Source: cosmos/cosmos-sdk

Summary

Remove alias.go files in favor of more verbose import names such as <modulename>types and <modulename>keeper

Problem Definition

alias.go can be useful when managing access to internal types, but the sdk does not use internals anymore. Currently I believe its usage is purely aesthetic.

I think verbose import names are especially useful for new developers to the codebase who might not be familiar with the differences between keeper/ and types/. Verbose import names indicate clearly which package the type comes from. I've also found alias.go to be out dated do to lack of enforcement of updating it on unused types (perhaps from the types being directly imported).

Proposal

Remove alias.go, use verbose import names. Remove from module design spec as well.


For Admin Use

  • [ ] Not duplicate issue
  • [ ] Appropriate labels applied
  • [ ] Appropriate contributors tagged
  • [ ] Contributor assigned/self-assigned
api-breaking help wanted meta-issue proposal-accepted

Most helpful comment

I will track the progress of this task here:

| module | status| PR
|---|-----|----|
| auth | closed |#6440|
| bank | closed |#6311|
| capability | closed|#6438|
| crisis | closed |#6437|
| distribution | closed |#6436 |
| evidence | closed |#6435|
| genaccounts | closed| no needed |
| genutil | closed| #6433|
| gov | closed |#6432|
| ibc | closed|#6429|
| ibc-transfer | closed | #6427|
| mint | closed |#6424 |
| params | closed |#6415|
| simulation | closed | no needed |
| slashing |closed |#6417|
| staking | closed | #6397|
| upgrade | closed |#6382|

All 9 comments

I fully support this proposal as I was never a fan of the alias approach. Now that we no longer have internal APIs, this makes more sense. We may want to revisit this post software freeze/1.0 stable release.

Hi, can I work on this?

Sure thing @dauTT. I’d recommend you to do it one module per PR at a time. Otherwise the changes would be massive

I will track the progress of this task here:

| module | status| PR
|---|-----|----|
| auth | closed |#6440|
| bank | closed |#6311|
| capability | closed|#6438|
| crisis | closed |#6437|
| distribution | closed |#6436 |
| evidence | closed |#6435|
| genaccounts | closed| no needed |
| genutil | closed| #6433|
| gov | closed |#6432|
| ibc | closed|#6429|
| ibc-transfer | closed | #6427|
| mint | closed |#6424 |
| params | closed |#6415|
| simulation | closed | no needed |
| slashing |closed |#6417|
| staking | closed | #6397|
| upgrade | closed |#6382|

nice table! To make things easier you don't need to create issues for each one. You can just ref this issue and close it when they are all done.

Great 👍 . One thing I often encounter with aliases is that they introduce unnecessary cyclic dependencies sometimes. Often, I have to manually use x/auth/types instead of x/auth to avoid this.

All the sub tasks above are completed. I think we can close this issue. :smile:

Sub modules in IBC need to have alias.go removed (03-connection, 04-channel etc)

there seems to be one last straggler: https://github.com/cosmos/cosmos-sdk/blob/bf8809ef9840b4f5369887a38d8345e2380a567f/x/auth/vesting/alias.go#L1-L1 if anyone wants to pick it up

Was this page helpful?
0 / 5 - 0 ratings

Related issues

cwgoes picture cwgoes  ·  3Comments

rigelrozanski picture rigelrozanski  ·  3Comments

johnmcdowall picture johnmcdowall  ·  3Comments

ValarDragon picture ValarDragon  ·  3Comments

rigelrozanski picture rigelrozanski  ·  3Comments