-
Notifications
You must be signed in to change notification settings - Fork 144
RUST-503 Support serializing directly to Document #210
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
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.
It seems like it might be worth it to add from_document
as well for parity (and letting users avoid having to manually wrap in Bson::Document
and calling from_bson
). That would also let us test basic round-tripping.
src/ser/mod.rs
Outdated
T: Serialize, | ||
{ | ||
let bson = to_bson(value)?; | ||
bson.as_document() |
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 think it would be more efficient to just pattern match the value and return it directly instead of cloning it. I know that using combinators is more fun though, but it's worth it to keep things as quick as possible for users :/
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.
looks good, one little nit about the error message
RUST-503