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

made set and get take AsRef<str> #36

Merged
merged 4 commits into from
Apr 18, 2023
Merged

Conversation

PhaestusFox
Copy link
Contributor

made the set and get methods on the PKVStore take AsRef so that the user can use types the impl AsRef as an alternative to just &str, this means you can use an enum and strum to quickly make a set of keys and no have to worry about spelling mistakes, this also doesn't lock the user into a single type like a generic would and shouldn't break and currently used cases outside of people using calls to into() or as_ref() to get the same behaviour

made the set and get methods on the PKVStore take AsRef<str> so that the user can use types the impl AsRef<str> as an alternative to just &str, this mean you can use and enum and strum to quickly make a key set and no have to worry about spelling mistakes, this also doesn't lock the used into a single type like a generic would and shouldn't break and current used cases outside of calls people using calls to into() or as_ref() to get the same behaviour
Copy link
Owner

@johanhelsing johanhelsing left a comment

Choose a reason for hiding this comment

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

I like it! :)

Some small nits on the example, and needs cargo fmt, but otherwise looks good :) Thanks!

examples/enumkeys.rs Outdated Show resolved Hide resolved
examples/enumkeys.rs Outdated Show resolved Hide resolved
examples/enumkeys.rs Outdated Show resolved Hide resolved
@johanhelsing
Copy link
Owner

Oh, and it needs an entry for the example with bevy as a required feature in cargo.toml (that's why the rocksdb ci tests fail)

@johanhelsing johanhelsing added the enhancement New feature or request label Apr 18, 2023
@johanhelsing
Copy link
Owner

Awesome :)

@johanhelsing johanhelsing merged commit eb9a9b9 into johanhelsing:main Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants