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

Color refactor #93

Merged
merged 1 commit into from
Aug 18, 2024
Merged

Color refactor #93

merged 1 commit into from
Aug 18, 2024

Conversation

dusk125
Copy link
Contributor

@dusk125 dusk125 commented Aug 13, 2024 •
edited
Loading

Refactor the pixel.RGBA to be an alias of color.RGBA so that it's less data to move around and better integrated with the standard library.

Also, adds an implicit contract with the caller.

  • If a function takes a color, it should be a color.Color.
  • If a function returns a color, it will be a pixel.RGBA (alias for color.RGBA.

Addresses: #71

@dusk125 dusk125 changed the title Color refactor [BREAKING] Color refactor Aug 13, 2024
@hajimehoshi
Copy link

Just out of curiosity, are you adding breaking changes without changing the major version from 2?

@dusk125
Copy link
Contributor Author

dusk125 commented Aug 13, 2024

Just out of curiosity, are you adding breaking changes without changing the major version from 2?

No, I marked this as breaking so we would know that we would need to increase the major version, and to see if this change is something we actually want to consider.

I have some other refactoring I want to consider as well, so for the non-draft version of this PR, I'll probably create a v3 branch to accumulate these breaking changes before moving the project to v3.

@dusk125 dusk125 added this to the v3 milestone Aug 13, 2024
@dusk125 dusk125 mentioned this pull request Aug 14, 2024
Open
@dusk125 dusk125 changed the title [BREAKING] Color refactor Color refactor Aug 14, 2024
@dusk125 dusk125 changed the base branch from main to v3-wip August 14, 2024 02:42
@dusk125 dusk125 marked this pull request as ready for review August 14, 2024 02:42
@dusk125 dusk125 requested a review from bhperry August 14, 2024 02:42
@dusk125 dusk125 self-assigned this Aug 14, 2024
@dusk125 dusk125 force-pushed the color-refactor branch 4 times, most recently from 888247d to 7ddf133 Compare August 15, 2024 21:41
Refactor out pixel.RGBA
c8b1b4c 
Stuff

Revert "Refactor out pixel.RGBA"

This reverts commit d029ce8.

Reworked to keep pixel.RGBA around, but an alias

working on color

more color work
@duysqubix
Copy link
Contributor

Just out of curiosity, are you adding breaking changes without changing the major version from 2?

We did go from v1 to v2 and have the original v1 snapshot as a release.

@duysqubix duysqubix marked this pull request as draft August 16, 2024 16:27
@dusk125
Copy link
Contributor Author

dusk125 commented Aug 16, 2024

And just a note, this PR is now targeting a v3 feature branch instead of main.

@duysqubix duysqubix requested review from a team and removed request for bhperry August 16, 2024 16:28
@duysqubix duysqubix marked this pull request as ready for review August 16, 2024 16:30
duysqubix
duysqubix approved these changes Aug 16, 2024
View reviewed changes
Copy link
Contributor

@duysqubix duysqubix 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.

looks good

bhperry
bhperry reviewed Aug 16, 2024
View reviewed changes
@dusk125 dusk125 merged commit 9ba93aa into gopxl:v3-wip Aug 18, 2024
@dusk125 dusk125 deleted the color-refactor branch August 18, 2024 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhperry bhperry bhperry left review comments

@duysqubix duysqubix duysqubix approved these changes

Assignees

@dusk125 dusk125

Labels
None yet
Projects
None yet
Milestone
v3
Development

Successfully merging this pull request may close these issues.
4 participants
@dusk125 @hajimehoshi @duysqubix @bhperry