Cosmos-sdk: Types Folder as an Alias Hub, not a types hub

Created on 2 Jun 2018  路  5Comments  路  Source: cosmos/cosmos-sdk

Rather than having the types directory be a mish mash of all the possible types which are defined in other packages (typically either root sdk packages or x/ packages) types should be reorganized in the following way:

  • if the type is intended to be defined from modules in x/ (can't be statically depended on) in keep them as in the types folder
  • if the type is defined elsewhere in the sdk but we just wanted to be easily accessible to the to anybody importing the sdk (so we defined it in types so they could just reference the one folder) move the definition of that type to the (non-x/) package where it's implementations functionality is defined, and then create and alias to it in the types/ directory. This way as a new user you can still simply import types/- however the definition of the type is now grouped with it's implementation (neater)
  • if the type (in types/) has substantial associated logic (such as rational IMO) right there in the types directory a good practice could be to move it to a new package, maybe as a subpackage in types, and then alias it just like the previous bullet.

CC @jaekwon @xla @ebuchman @cwgoes @sunnya97

EDIT: Another consideration - "loose" helper functions sitting in types such as PrefixEndBytes in types/store.go should be moved to their categorical package and then referenced with a function alias - this particular example:

  • types/store.go/PrefixEndBytes moves to store/helpers.go/PrefixEndBytes
  • types/store.go adds:

    • var PrefixEndBytes = store.PrefixEndBytes

kapoosh!

Legacy API baseapp code-hygiene

Most helpful comment

summarizing a discussion with @xla

There are really three elements which need to be considered:

  • SDK (./ not incl /x, /cmd, /examples)
  • Modules (/x) consumers of: SDK
  • Applications (/cmd, /examples) consumers of: SDK, modules

we came to the conclusion that we should:

  • Have the aliases on the root of cosmos-sdk not in the types package - this will effectively act as the types package used to
  • The root package name should be cosmos to bring some context for third party packages importing the cosmos-sdk repo
  • consumers of the SDK (aka modules/applications) will import only from the aliases for ease
  • internally with the SDK the aliases are not used, and the specific sub-packages should be referenced

All 5 comments

if the type is defined elsewhere in the sdk but we just wanted to be easily accessible to the to anybody importing the sdk (so we defined it in types so they could just reference the one folder) move the definition of that type to the (non-x/) package where it's implementations functionality is defined, and then create and alias to it in the types/ directory. This way as a new user you can still simply import types/- however the definition of the type is now grouped with it's implementation (neater)

Is the sole motivation reduction of import statements for integrators? Referencing the Go Code Review Comments on package names here. There was an internal discussion on this before, just to surface it again, what are the requirements and needs from integrators (how many consumers of the SDK exists) we optimise for here?

Is the sole motivation reduction of import statements for integrators?

Yeah a large motivation of the types packages is to make it dead-simple straight forward to import any public type - shouldn't have to think about this as a developer I don't think.

Referencing the Go Code Review Comments on package names here

Yeah I'm also aware of the merit of limiting the variable definition length through the use of separated packages (more imports) as the link points out - it's really just a tradeoff with number of imports. But yeah also it's not a huge issue for the sdk, browsing through the types packages it's mostly just the NewFooBar functions which are longer than they would need to be.

summarizing a discussion with @xla

There are really three elements which need to be considered:

  • SDK (./ not incl /x, /cmd, /examples)
  • Modules (/x) consumers of: SDK
  • Applications (/cmd, /examples) consumers of: SDK, modules

we came to the conclusion that we should:

  • Have the aliases on the root of cosmos-sdk not in the types package - this will effectively act as the types package used to
  • The root package name should be cosmos to bring some context for third party packages importing the cosmos-sdk repo
  • consumers of the SDK (aka modules/applications) will import only from the aliases for ease
  • internally with the SDK the aliases are not used, and the specific sub-packages should be referenced

So we _can_ actually keep the repo-name as Cosmos-Sdk and just name the package cosmos and then imports will accept the cosmos as the import by default (even without an alias in the import)

After having a convo with @jaekwon about this issue - we decided that this is a minor priority for the time being, however there are a couple offshoot issues #3928 #3929

It would be really great if we had the opportunity to import from the repo level without requiring a giant alias file... I don't think go allows this though

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mossid picture mossid  路  3Comments

fedekunze picture fedekunze  路  3Comments

ValarDragon picture ValarDragon  路  3Comments

hendrikhofstadt picture hendrikhofstadt  路  3Comments

rigelrozanski picture rigelrozanski  路  3Comments