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

add serde feature and implement Serialize and Deserialize on most error types #33

Merged
merged 9 commits into from
Jun 21, 2024

Conversation

Arne91
Copy link
Contributor

@Arne91 Arne91 commented Feb 28, 2024

To serialize and deserialize the ParseError, the crate "serde" is added.

@felixwrt
Copy link
Owner

Thanks for taking the time to contribute. I'm ok with adding serde implementations to the code base, but they should be optional. Could you change that, please? There are lots of crates that have an optional serde feature, so it shouldn't be difficult to find inspiration. Other than that, please also add the new feature to the crate's documentation. Thanks in advance!

@Arne91
Copy link
Contributor Author

Arne91 commented Mar 1, 2024

Thanks for taking the time to contribute. I'm ok with adding serde implementations to the code base, but they should be optional. Could you change that, please? There are lots of crates that have an optional serde feature, so it shouldn't be difficult to find inspiration. Other than that, please also add the new feature to the crate's documentation. Thanks in advance!

Thank you for your quick feedback. That is a good point.
I added it as optional "serde" feature. I also added it in the crate docs.

@felixwrt felixwrt changed the title add serde for ParseError add serde and implement Serialize and Deserialize on most error types Jun 21, 2024
@felixwrt felixwrt changed the title add serde and implement Serialize and Deserialize on most error types add serde feature and implement Serialize and Deserialize on most error types Jun 21, 2024
@felixwrt felixwrt merged commit 5f6aac5 into felixwrt:main Jun 21, 2024
18 checks passed
@felixwrt
Copy link
Owner

I'm sorry that it's been taking me so long to integrate this - I've now finally managed to do so. Thanks again for contributing and let me know if there's more you'd like to see changed.

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.

2 participants