-
Notifications
You must be signed in to change notification settings - Fork 192
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
Added conversion for Vec<u8> #699
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @Jeremiah-Griffin!
We’ll need to make this impl conditional so that no-std users don’t get it.
Right now we have a std
feature that would cover it. If you’re keen though you could add an alloc
feature that’s implied by std
and just brings in the alloc
crate.
use crate::{ | ||
std::{borrow::Borrow, fmt, ptr, str}, | ||
Uuid, Variant, | ||
}; | ||
|
||
impl From<Uuid> for Vec<u8>{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to be conditionally compiled, with a #[cfg(feature = “std”])
.
@@ -272,6 +272,7 @@ mod macros; | |||
#[doc(hidden)] | |||
#[cfg(feature = "macro-diagnostics")] | |||
pub extern crate uuid_macro_internal; | |||
extern crate alloc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will also need to be conditionally compiled.
} | ||
} | ||
|
||
impl From<Vec<u8>> for Uuid{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should add this side of the conversion. Converting slices into Uuid
s is fallible, but we can't express that through this trait.
@@ -70,6 +70,9 @@ fast-rng = ["rng", "rand"] | |||
sha1 = ["sha1_smol"] | |||
md5 = ["md-5"] | |||
|
|||
[dependencies] | |||
diesel = {version = "2.1.0", features = ["sqlite"]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, digging back into this it looks like diesel
actually already depends on uuid
, so I don't think we can add it as a dependency here. You might not actually need to do anything in this library to use it with diesel
besides enable its uuid
feature. It looks like their support is implemented here: https://github.com/diesel-rs/diesel/blob/a122e7b88a1bc1b427dde34ed36ce3f36fad772e/diesel/src/pg/types/uuid.rs#L13.
Thanks again for the PR @Jeremiah-Griffin. I've ended up merging #703 which adds some |
Hello,
An
impl From<Uuid> for Vec<u8>
is quite useful for crates like Diesel which need such a conversion to work with its codegen of database models.