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.

Sign up for GitHub

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

Jump to bottom

FIX12527: Changes to make serde optional for bevy_color #12666

Merged
merged 3 commits into from
Mar 24, 2024
Merged

FIX12527: Changes to make serde optional for bevy_color #12666

merged 3 commits into from
Mar 24, 2024

Conversation

mghildiy
Copy link
Contributor

Objective

Solution

  • Added feature for serialization

Changelog

  • Serde serialization is now optional, with flag 'serialize'

Migration Guide

  • If user wants color data structures to be serializable, then application needs to be build with flag 'serialize'

@github-actions GitHub Actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@mghildiy mghildiy mentioned this pull request Mar 23, 2024
Closed
@ItsDoot ItsDoot added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Utils Utility functions and types labels Mar 23, 2024
@james7132 james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 23, 2024
@alice-i-cecile
Copy link
Member

Like CI says, you need to run cargo fmt --all on your code. Don't worry about the Dependencies job though: it's flaky and non blocking.

pablo-lua
pablo-lua suggested changes Mar 23, 2024
View reviewed changes
Copy link
Contributor

@pablo-lua pablo-lua left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks!. Nothing to say about the feature itself, but we can probably improve some lines

crates/bevy_color/src/color.rs Outdated Show resolved Hide resolved
crates/bevy_color/src/hsla.rs Outdated Show resolved Hide resolved
crates/bevy_color/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/camera/clear_color.rs Outdated Show resolved Hide resolved
@pablo-lua pablo-lua added the A-Rendering Drawing game state to the screen label Mar 23, 2024
@alice-i-cecile alice-i-cecile removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 23, 2024
@alice-i-cecile
Copy link
Member

Going to wait for some cleanup before merging this :)

@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Mar 23, 2024
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 24, 2024
pablo-lua
pablo-lua suggested changes Mar 24, 2024
View reviewed changes
Copy link
Contributor

@pablo-lua pablo-lua left a comment

Choose a reason for hiding this comment

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

Just some problems with reflection import here, otherwise, everything is fine

crates/bevy_color/src/color.rs Outdated Show resolved Hide resolved
crates/bevy_color/src/oklaba.rs Outdated Show resolved Hide resolved
pablo-lua
pablo-lua approved these changes Mar 24, 2024
View reviewed changes
Copy link
Contributor

@pablo-lua pablo-lua left a comment

Choose a reason for hiding this comment

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

All good! LGTM

@james7132 james7132 added this pull request to the merge queue Mar 24, 2024
Merged via the queue into bevyengine:main with commit 9e09707 Mar 24, 2024
30 checks passed
@mghildiy mghildiy deleted the fix12527 branch March 24, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@james7132 james7132 james7132 approved these changes

@ItsDoot ItsDoot ItsDoot approved these changes

@pablo-lua pablo-lua pablo-lua approved these changes

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen A-Utils Utility functions and types C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
0.14
Development

Successfully merging this pull request may close these issues.

Add serialize feature to bevy_color
5 participants
@mghildiy @alice-i-cecile @james7132 @ItsDoot @pablo-lua