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

style/color: derive serialize & deserialize #659

Merged

Conversation

10ne1
Copy link
Contributor

@10ne1 10ne1 commented Nov 1, 2024

Using serde is useful in production, not just during development, so move it to optional dependencies instead of dev-dependencies.

This allows to derive serialize/deserialize for the color structs which are very simple using the default implementation, allowing to pass colors along via serialization channels and formats.

@10ne1 10ne1 force-pushed the dev/aratiu/allow-serializing-colors branch from 3df125c to 030a21a Compare November 1, 2024 15:40
Copy link
Member

@AaronErhardt AaronErhardt left a comment

Choose a reason for hiding this comment

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

I totally understand the intend of the PR and I agree that this is useful. However, some might want to use plotters without pulling in serde as dependency. Therefore, this feature should be added behind a feature flag IMO.

@10ne1 10ne1 force-pushed the dev/aratiu/allow-serializing-colors branch from 030a21a to 5eb2238 Compare December 4, 2024 08:21
Using serde is useful in production, not just during development,
so move it to optional dependencies instead of dev-dependencies.

This allows to derive serialize/deserialize for the color structs
which are very simple using the default implementation, allowing
to pass colors along via serialization channels and formats.
@10ne1 10ne1 force-pushed the dev/aratiu/allow-serializing-colors branch from 5eb2238 to 7f96558 Compare December 4, 2024 08:51
@10ne1
Copy link
Contributor Author

10ne1 commented Dec 4, 2024

Hello @AaronErhardt and thank you for your patience!

I've updated the PR to make serde an optional dependency while still keeping it enabled in dev-dependencies and added a small documentation blurb for the new serialization feature.

Copy link
Member

@AaronErhardt AaronErhardt left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

@AaronErhardt AaronErhardt merged commit 3ad52da into plotters-rs:master Dec 6, 2024
18 checks passed
@10ne1 10ne1 deleted the dev/aratiu/allow-serializing-colors branch December 6, 2024 12:47
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