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

Chore: update base enum class with forked version from myclabs #7383

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

jonwaldstein
Copy link
Contributor

@jonwaldstein jonwaldstein commented May 3, 2024

Resolves: GIVE-760

Description

We have a php 8.1 incompatibility conflict with the myclabs/php-enum class. Unfortunately we are currently tied to php 7.2 at the moment and cannot update to their latest package that resolves the conflict.

Return type of MyCLabs\Enum\Enum::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in vendor/myclabs/php-enum/src/Enum.php on line 246

Furthermore, this package is not really maintained anymore since enums were introduced in php 8.1

So for more control over stability and compatibility with GiveWP, this PR forks the base enum class from myclabs including the unit tests and updates our Enum to extend it.

Affects

This technically affects our value objects that extend the enum class, however it's still the same logic with the namespace being our own so the risk is very low.

Visuals

N/A

Testing Instructions

Unit tests and static analysis should take care of the bulk testing. We will want to run through QA regression testing as well.

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

@jonwaldstein jonwaldstein changed the title Chore: update enum version and load with Strauss Chore: update base enum class with forked version from myclabs May 3, 2024
@jonwaldstein jonwaldstein marked this pull request as ready for review May 8, 2024 20:07
Copy link
Contributor

@JasonTheAdams JasonTheAdams left a comment

Choose a reason for hiding this comment

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

Ah, makes sense. Left one very minor suggestion so I'll approve.

src/Framework/Support/ValueObjects/BaseEnum.php Outdated Show resolved Hide resolved
Copy link
Member

@rickalday rickalday left a comment

Choose a reason for hiding this comment

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

Passed QA tests

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.

None yet

3 participants