Skip to content

[Serializer] Add support of PHP backed enumerations #40830

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
Merged
merged 1 commit into from
Jul 5, 2021
Merged

[Serializer] Add support of PHP backed enumerations #40830

merged 1 commit into from
Jul 5, 2021

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Apr 15, 2021 •
edited
Loading

Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #40241
License MIT
Doc PR none

Capture d’écran du 2021-04-15 21-41-51
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch from 2b935bc to 894f1d7 Compare April 15, 2021 20:05
jderusse
jderusse reviewed Apr 15, 2021
View reviewed changes
Copy link
Member

@jderusse jderusse left a comment

Choose a reason for hiding this comment

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

Why only BackedEnum? can't we also normalize Enum via Reflexion?
OskarStark
OskarStark reviewed Apr 15, 2021
View reviewed changes
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch from 894f1d7 to 0419400 Compare April 15, 2021 20:30
@alexandre-daubois
Copy link
Member Author

alexandre-daubois commented Apr 15, 2021 •
edited
Loading

Thank you for your reviews!

Why only BackedEnum? can't we also normalize Enum via Reflexion?

@stof commented in the original ticket #40241 that we should only create this normalizer for BackedEnum, as serializing pure enum case would be better with a custom normalizer. Do you think that it would be better to create some UnitEnumNormalizer too?

I would go with 5.4 but we need to add a php version id check somewhere monocle_face

Edit: added version check in d8298fa
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch from 0419400 to d8298fa Compare April 16, 2021 06:39
stof
stof requested changes Apr 16, 2021
View reviewed changes
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch 6 times, most recently from 29b08fe to 7d9709e Compare April 16, 2021 13:21
stof
stof requested changes Apr 16, 2021
View reviewed changes
@stof stof mentioned this pull request Apr 16, 2021
Closed
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch from 7d9709e to 0a07caa Compare April 16, 2021 16:50
@nicolas-grekas nicolas-grekas added this to the 5.x milestone Apr 16, 2021
@alexandre-daubois alexandre-daubois mentioned this pull request Apr 19, 2021
Merged
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch from 0a07caa to 6e01a7a Compare April 19, 2021 14:25
@alexandre-daubois
Copy link
Member Author

Changed @requires PHP 8.1 annotations after @derrabus comment on #40857 (comment)
@alexandre-daubois alexandre-daubois changed the title [Serializer] Add support of PHP 8.1 backed enumerations [Serializer] Add support of PHP backed enumerations Jun 4, 2021
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch from e7c1a97 to ca6abf6 Compare June 4, 2021 06:44
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch from ca6abf6 to 0872747 Compare June 23, 2021 05:21
@alexandre-daubois alexandre-daubois changed the base branch from 5.3 to 4.4 June 23, 2021 05:21
@alexandre-daubois
Copy link
Member Author

Rebased to 4.4, as it is considered a bug fix. How to deal with the change of configuration format, going from XML to PHP configuration in latest Symfony version?
@nicolas-grekas
Copy link
Member

Thanks. We'll deal with it while merging up.
derrabus
derrabus approved these changes Jun 23, 2021
View reviewed changes
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch from 0872747 to e9ed1e8 Compare June 23, 2021 09:25
fabpot
fabpot reviewed Jul 2, 2021
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

As it adds a new class to support it, I would say this is out of the "merge in 4.4 to support a new PHP version". I think it qualifies as a new feature, so for 5.4.
@alexandre-daubois alexandre-daubois changed the base branch from 4.4 to 5.4 July 5, 2021 11:22
@alexandre-daubois alexandre-daubois force-pushed the feat-backed-enum-normalizer branch from e9ed1e8 to 3458e0e Compare July 5, 2021 11:22
@alexandre-daubois alexandre-daubois changed the base branch from 5.4 to 4.4 July 5, 2021 11:26
@alexandre-daubois alexandre-daubois changed the base branch from 4.4 to 5.4 July 5, 2021 11:28
@alexandre-daubois
Copy link
Member Author

Alright! Rebase to 5.4 is done
@fabpot
Copy link
Member

fabpot commented Jul 5, 2021

Thank you @alexandre-daubois.
@fabpot fabpot mentioned this pull request Jul 5, 2021
Closed
@fabpot fabpot merged commit 61f26b6 into symfony:5.4 Jul 5, 2021
@ro0NL ro0NL mentioned this pull request Sep 16, 2021
Closed
This was referenced Nov 5, 2021
Merged
Merged
@GuilhemN GuilhemN mentioned this pull request Feb 28, 2022
Merged
@nicolas-grekas nicolas-grekas mentioned this pull request Aug 17, 2022
Merged
nicolas-grekas added a commit that referenced this pull request Aug 17, 2022
@nicolas-grekas
minor #47298 Add missing types to BackedEnumNormalizer (nicolas-grekas)
3a90e4c 
This PR was merged into the 5.4 branch. Discussion ---------- Add missing types to BackedEnumNormalizer | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Spotted while reviewing #47296 Not sure why we missed adding them in #40830 🤷 Commits ------- 60d6325 [Serializer] Add missing types to BackedEnumNormalizer
@TerjeBr TerjeBr mentioned this pull request Apr 28, 2023
Merged
@TerjeBr TerjeBr mentioned this pull request Sep 12, 2023
Open
@soyuka soyuka mentioned this pull request Apr 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature Serializer Status: Reviewed

10 participants

@alexandre-daubois @nicolas-grekas @fabpot @dunglas @stof @jderusse @OskarStark @derrabus @maxhelias @carsonbot