-
Notifications
You must be signed in to change notification settings - Fork 134
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
RUST-1699: Add serde_with 3.x integration #422
Conversation
Cargo.toml
Outdated
@@ -65,13 +65,13 @@ lazy_static = "1.4.0" | |||
uuid-0_8 = { package = "uuid", version = "0.8.1", features = ["serde", "v4"], optional = true } | |||
uuid = { version = "1.1.2", features = ["serde", "v4"] } | |||
serde_bytes = "0.11.5" | |||
serde_with = { version = "1", optional = true } | |||
serde_with1 = { package = "serde_with", version = "1.3.1", optional = true } |
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 we need to keep the existing one without renaming it so that existing users turning on the serde_with
feature will continue to get the same version.
src/uuid/mod.rs
Outdated
@@ -509,6 +509,30 @@ macro_rules! trait_impls { | |||
uuid.serialize(serializer) | |||
} | |||
} | |||
|
|||
#[cfg(all($feat, feature = "serde_with3"))] |
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 this (and below) need to be feature = "serde_with-3"
to match Cargo.toml.
Co-authored-by: Abraham Egnor <[email protected]>
README.md
Outdated
@@ -50,8 +50,8 @@ Note that if you are using `bson` through the `mongodb` crate, you do not need t | |||
| `chrono-0_4` | Enable support for v0.4 of the [`chrono`](https://docs.rs/chrono/0.4) crate in the public API. | n/a | no | | |||
| `uuid-0_8` | Enable support for v0.8 of the [`uuid`](https://docs.rs/uuid/0.8) crate in the public API. | n/a | no | | |||
| `uuid-1` | Enable support for v1.x of the [`uuid`](https://docs.rs/uuid/1.0) crate in the public API. | n/a | no | | |||
| `serde_with` | Enable [`serde_with`](https://docs.rs/serde_with/latest) integrations for `bson::DateTime` and `bson::Uuid` | serde_with | no | | |||
|
|||
| `serde_with` | Enable [`serde_with`](https://docs.rs/serde_with/latest) 1.x integrations for `bson::DateTime` and `bson::Uuid`.| serde_with | no | |
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'm doing a terrible job reviewing for which I apologize! Reading this made me go look and there are also implementations for crate::DateTime
which will need serde_with_3
versions as well.
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.
LGTM!
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.
just a few suggestions! we also have documentation sections about serde_with
for DateTime
and Uuid
. I think we should update the references to serde_with
to be serde_with-3
instead to encourage users to be on the latest version. Something like:
The
serde_with-3
feature (orserde_with
feature for version 1.x) can be enabled...
Co-authored-by: Isabel Atkinson <[email protected]>
Co-authored-by: Isabel Atkinson <[email protected]>
Co-authored-by: Isabel Atkinson <[email protected]>
Added
serde_with
3.x support. Docs