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

Make all structs serializable #19

Closed
wants to merge 1 commit into from

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