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

Backed enum value changed to Atomic instead of scalar int or strings #10165

Merged
merged 5 commits into from
Aug 31, 2023

Conversation

tuqqu
Copy link
Contributor

@tuqqu tuqqu commented Aug 30, 2023

Closes #9938

@tuqqu tuqqu marked this pull request as draft August 30, 2023 23:18
@tuqqu tuqqu force-pushed the backed-enum-value-changed-to-atomic branch 2 times, most recently from a99da7c to 06a3a02 Compare August 30, 2023 23:30
@tuqqu tuqqu marked this pull request as ready for review August 30, 2023 23:39
@weirdan
Copy link
Collaborator

weirdan commented Aug 31, 2023

Since there are no BC breaks, you can target 5.x instead of master.

@tuqqu tuqqu changed the base branch from master to 5.x August 31, 2023 17:04
@tuqqu tuqqu force-pushed the backed-enum-value-changed-to-atomic branch from 4f95db3 to 5ba7c26 Compare August 31, 2023 17:06
@tuqqu
Copy link
Contributor Author

tuqqu commented Aug 31, 2023

@weirdan the target branch is changed

@weirdan weirdan changed the base branch from 5.x to master August 31, 2023 18:24
@weirdan
Copy link
Collaborator

weirdan commented Aug 31, 2023

the target branch is changed

On second thought, the following would break:

fn(EnumCaseStorage $s): int|string|null { return $s->value; }

It looks like I relied on automated checks too much. I've changed the target branch back to master, sorry about this confusion.

Comment on lines 67 to 68
if ($enum_case_storage->value instanceof TLiteralString
|| $enum_case_storage->value instanceof TLiteralInt) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if ($enum_case_storage->value instanceof TLiteralString
|| $enum_case_storage->value instanceof TLiteralInt) {
if ($enum_case_storage->value !== null) {

@weirdan weirdan added the release:feature The PR will be included in 'Features' section of the release notes label Aug 31, 2023
@tuqqu tuqqu force-pushed the backed-enum-value-changed-to-atomic branch from 9312fc3 to 76f03cc Compare August 31, 2023 18:44
@weirdan weirdan merged commit c50e822 into vimeo:master Aug 31, 2023
39 checks passed
@weirdan
Copy link
Collaborator

weirdan commented Aug 31, 2023

Thanks!

@tuqqu tuqqu deleted the backed-enum-value-changed-to-atomic branch August 31, 2023 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature The PR will be included in 'Features' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enum values as class strings
2 participants