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

feat: serde serialization/deserialization support #20

Closed
wants to merge 5 commits into from

Conversation

EstebanBorai
Copy link
Owner

Provides an optional feature to support serde for serializing
and deserializing NetworkInterface, Addr, V4IfAddr, V6IfAddr.

Resolves: #19

Provides an optional feature to support `serde` for serializing
and deserializing `NetworkInterface`, `Addr`, `V4IfAddr`, `V6IfAddr`.

Resolves: #19
Cargo.toml Outdated
@@ -13,6 +13,7 @@ readme = "README.md"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
serde = { version = "1.0.144", features = ["derive"], optional = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

In general when adding serde we try to avoid including the derive feature as it's lead to slower compile time. See this this section about derive in clap documentation about this.

Without the derive feature, we have to impl Serialize by hand. I think we can just copy and paste the code generated by the #[derive(Serialize)] macro. To see the generated code we can use cargo expand.

There are many example of crate that add a serde feature without the derive, for instance Uuid or the time crate.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks for the suggestions! I agree with you, let me apply changes and get back to you.

@EstebanBorai
Copy link
Owner Author

@Tudyx sorry about the delay! I just pushed tests for serialization.
Now I'm moving into deserialization. If you have any insights/feedback please don't hesitate on sharing!

@Tudyx
Copy link
Contributor

Tudyx commented Sep 21, 2022

Seems great to me so far, well done

@HuakunShen
Copy link
Contributor

@EstebanBorai Any update on this? Really need this feature.

The current release doesn't seem to support serialize or deserialize.

@EstebanBorai
Copy link
Owner Author

@EstebanBorai Any update on this? Really need this feature.

The current release doesn't seem to support serialize or deserialize.

Hi @HuakunShen!

I have not worked on this for a while. The goal was to speed up compile times by implementing the trait manually over using derive.

I think we can merge your PR and work on this later as an optimization.

Thanks so much!

@EstebanBorai EstebanBorai deleted the feat/serde branch August 15, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for serde
3 participants