Skip to content

Implement KV store API, add testing suite#70

Merged
zkat merged 20 commits intomainfrom
sy/kv_store
Sep 4, 2025
Merged

Implement KV store API, add testing suite#70
zkat merged 20 commits intomainfrom
sy/kv_store

Conversation

@TartanLlama
Copy link
Copy Markdown
Collaborator

The testing suite just covers the KV store API for now, more to come in a future PR

@TartanLlama TartanLlama requested a review from zkat September 1, 2025 09:40
@TartanLlama
Copy link
Copy Markdown
Collaborator Author

Fixes #46 and #33

Copy link
Copy Markdown
Member

@zkat zkat left a comment

Choose a reason for hiding this comment

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

This is so great. This was a pretty big API to tackle, and the stuff you've been doing made it look so clean. Also! We have tests now! what!!

Comment thread test/kv_store.cpp
}

SECTION("erase") {
REQUIRE(store.insert(key, std::move(value_body)));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I don't understand std::move enough here. How are you moving the same value multiple times?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The SECTIONs are run separately and the code at the top level of the TEST_CASE is run once for each of them, so the value_bodys in each SECTION are distinct

Comment thread src/lib.rs
) -> bool;
// Needed to force generation of `drop`.
fn f_header_values_iter_noop(val: Box<HeaderValuesIter>) -> Box<HeaderValuesIter>;
fn f_header_values_iter_force_symbols(val: Box<HeaderValuesIter>) -> Box<HeaderValuesIter>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is a much better name fr

@zkat zkat added feature New feature or request technical improvement Non-user-visible technical improvement labels Sep 4, 2025
@zkat zkat merged commit acc11f4 into main Sep 4, 2025
6 checks passed
@zkat zkat deleted the sy/kv_store branch September 4, 2025 15:41
@TartanLlama TartanLlama restored the sy/kv_store branch September 5, 2025 09:08
@zkat zkat mentioned this pull request Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request technical improvement Non-user-visible technical improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants