Skip to content

Conversation

omarshehab221
Copy link
Contributor

Making all structs serializable makes them usable in API Requests and IPC communication.

@jeremychone
Copy link
Owner

@omarshehab221 Very cool. Were you the one who left the comment on one of my YouTube videos?

Let me take a look at this PR.

At first glance, it looks good. However, I’m not sure I want to implement serialize/deserialize for things that aren’t exposed outside of the crate. So, I’ll probably accept/merge this PR and then make another commit to remove serialize/deserialize for anything that is internal only.

Make sense?

@jeremychone
Copy link
Owner

image

This could not compile, because reqwest::StatusCode. I will remove the serialize for this entity.

@omarshehab221
Copy link
Contributor Author

Yes, I was the one who made the comment.
You're probably right internal things don't need to be serialized. I'm still new to rust, but I'm trying my best.

@jeremychone
Copy link
Owner

@omarshehab221 I am merging it now. Fix couple of things as extra commits. Will be in main branch soon.

@jeremychone
Copy link
Owner

@omarshehab221 Ok, this has been merged. Somehow, github did not recognize the merge, but your commit is in, plus my edits.

@jeremychone jeremychone closed this Sep 1, 2024
@jeremychone jeremychone added the PR-MERGED Added for PR that somehow did not get the Merge label Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR-MERGED Added for PR that somehow did not get the Merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants