Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Further removal of templates in db classes #3785

Merged
merged 11 commits into from
Apr 11, 2022

Conversation

clemahieu
Copy link
Contributor

Moves methods off of store_partial and eliminates it.
Moves mdb_store/rocksdb_store in to their respective namespaces and renamed to store.

…ving function from store_partial.

This is one of the leaky abstractions resulting from the former templated code. lmdb exposes return statuses with int while rocksdb uses an enum class. Removing the leaky abstraction will allow this code to be specialized to the backend type and flow more logically.
…tics and this is how it's actually implemented.
@clemahieu clemahieu added this to the V24.0 milestone Apr 10, 2022
@clemahieu clemahieu requested review from thsfs and theohax April 10, 2022 20:45
@@ -27,7 +28,7 @@ bool nano::lmdb::account_store::get (nano::transaction const & transaction, nano
void nano::lmdb::account_store::del (nano::write_transaction const & transaction_a, nano::account const & account_a)
{
auto status = store.del (transaction_a, tables::accounts, account_a);
release_assert_success (store, status);
store.release_assert_success (status);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd let this as a free function.

Copy link
Contributor

@thsfs thsfs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@clemahieu clemahieu merged commit e4deda7 into nanocurrency:develop Apr 11, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants