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

Derive serde/schemars for various things #16

Closed
wants to merge 5 commits into from

Conversation

ratmice
Copy link
Contributor

@ratmice ratmice commented Oct 29, 2023

This is a companion patch for linebender/kurbo#321
So, if both repos believe it to be a good idea, and wish to merge them kurbo would need to get merged first.

@ratmice ratmice changed the title Derive serde/schemars for Style Derive serde/schemars for various things Oct 30, 2023
@ratmice
Copy link
Contributor Author

ratmice commented Oct 30, 2023

So, I'm really not familiar with the schemars crate or json schemas in general, so schemars derive for Brush still needs to be done. I don't imagine it's hard but, but doesn't have any adaptor feature for serde_bytes, and haven't managed to figure out deriving the trait manually as I didn't manage to get it working with derive macros.

@ratmice
Copy link
Contributor Author

ratmice commented Dec 7, 2023

The kurbo patches which were blocking this are now in, so this can perhaps make some progress.
In that spirit here are my notes on where this stands.

  • I haven't tested the Blob impl, because I only needed to add it to serialize other variants of the same enum.
  • BrushRef has a Serialize but no Deserialize impl, because you can't deserialize to a Non-Owned type
    Perhaps I should remove the Serialize impl from it?

Otherwise, been using it locally for a while.

Towards deriving these for `Brush`. Which I believe the only thing missing
needed for that is now `Blob`.
Because these impls depend on the following pr:
GREsau/schemars#252
@ratmice
Copy link
Contributor Author

ratmice commented Mar 15, 2024

I'm going to just close/reopen this PR for 2 reasons:

  • Mostly to just get it off of my peniko/main branch
  • Because initially this added both schemars and serde, but now just contains serde as schemars support is waiting on upstream review. Thus the new branch should just mention serde, which will simplify history...

@ratmice ratmice closed this Mar 15, 2024
@ratmice ratmice mentioned this pull request Mar 15, 2024
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.

1 participant