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

store: Suggested refactor #2309

Closed
ValarDragon opened this issue Sep 12, 2018 · 5 comments
Closed

store: Suggested refactor #2309

ValarDragon opened this issue Sep 12, 2018 · 5 comments
Assignees

Comments

@ValarDragon
Copy link
Contributor

ValarDragon commented Sep 12, 2018

Summary

Make all the store types exported, move each store into its own module.

Problem Definition / Proposed Solution

Currently its very hard to figure out how the internals of our stores works. This is partially due to lack of documentation, but also due to part of the way its structured. Currently no type is exported, so you wouldn't be aware of the type-specific pecularities, even if you wanted to. (Those godocs won't show up here: https://godoc.org/github.com/cosmos/cosmos-sdk/store, even though those methods are exported and satisfy the interface)
Thus I think we should export these types

I think moving each of these store types into their own module, with a doc.go file would be incredibly helpful. (Similar to what is done for every crypto package)

I have tagged this if-have-time-prelaunch, as I am very certain there are bugs in our store implementations, and it being poorly documented is likely a cause of it. Signs that there are bugs: RipeMD160 remained, there is an issue with iterator return values (either due to incorrect expectations from their names, or impl bugs)

@rigelrozanski
Copy link
Contributor

Yeah I'm totally into this - store should be moved into submodules that will increase the comprehensibility of the code bigtime

@mossid
Copy link
Contributor

mossid commented Sep 14, 2018

Working on it.

@jackzampolin
Copy link
Member

We could also do a docs fix here by adding documentation for the existing implementation. I think that portion could be prelaunch and the rest could be postlaunch. The first part of the store refactor has been hard to merge.

@alessio
Copy link
Contributor

alessio commented Jan 23, 2019

In my opinion, this is most definitely not blocking launch. Hence updating labels accordingly.

@jackzampolin
Copy link
Member

I think @mossid's store refactor addressed this. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants