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

split implementations to one per package #2

Closed
mvdan opened this issue Apr 17, 2020 · 2 comments
Closed

split implementations to one per package #2

mvdan opened this issue Apr 17, 2020 · 2 comments
Labels
chaindb Priority: 4 - Low Issue should be resolved eventually.

Comments

@mvdan
Copy link
Contributor

mvdan commented Apr 17, 2020

Right now this module is a single package, which is OK since we only have one implementation with badger.

However, it would be better long-term design to have the interface and generic code/tests at the root package, and one sub-package for each implementation that pulls in heavy dependencies like badger.

If we don't do that, we could easily end up in the case where trying to use badger also forces importing (and thus linking into the binary) other database software.

The first suggestion that comes to mind is chaindb/<dbname>, like chaindb/badger. However, this would be unfortunate as it clashes with the name of the upstream badger package itself, and it's entirely reasonable to want to use both at the same time (e.g. when using our package along with database options from upstream).

Another idea is to use slightly different names, like chaindb/badgerdb. Though that doesn't really avoid the confusion between the two names.

The real difference here is that our package is an implementation, or a wrapper around upstream. Perhaps chaindb/badgerimpl? It's a bit ugly, but I think it's the least confusing.

Another option is to just do chaindb/badger, and then let the importer rename it as they please like import badgerimpl "github.com/ChainSafe/chaindb/badger". Perhaps this is the simplest option.

@noot
Copy link
Contributor

noot commented Apr 22, 2020

I think the last option is probably the simplest and most straightforward.

@mvdan
Copy link
Contributor Author

mvdan commented Apr 22, 2020

I agree.

@dutterbutter dutterbutter added chaindb Priority: 4 - Low Issue should be resolved eventually. labels Jan 7, 2021
@mvdan mvdan closed this as not planned Won't fix, can't repro, duplicate, stale Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chaindb Priority: 4 - Low Issue should be resolved eventually.
Projects
None yet
Development

No branches or pull requests

3 participants