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

[Serializer] Add DateTimeNormalizer::CAST_KEY context option #53056

Merged
merged 1 commit into from
Feb 3, 2024
Merged

[Serializer] Add DateTimeNormalizer::CAST_KEY context option #53056

merged 1 commit into from
Feb 3, 2024

Conversation

norkunas
Copy link
Contributor

@norkunas norkunas commented Dec 13, 2023 •
edited
Loading

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #53055
License MIT

This adds DateTimeNormalizer::CAST_KEY context option allowing to cast the formatted result to int or float.

@carsonbot carsonbot added this to the 7.1 milestone Dec 13, 2023
@norkunas norkunas force-pushed the serializer-timestamp-normalization-cast branch from d45ef9a to fb671a2 Compare December 13, 2023 10:56
@norkunas norkunas force-pushed the serializer-timestamp-normalization-cast branch from fb671a2 to f2e8ba2 Compare December 15, 2023 05:55
@norkunas
Copy link
Contributor Author

@mtarld I've updated, is this what you had in mind with deprecation?

@norkunas norkunas force-pushed the serializer-timestamp-normalization-cast branch 5 times, most recently from 39809e2 to aa8cf48 Compare December 15, 2023 06:26
@norkunas norkunas requested a review from mtarld December 15, 2023 06:29
@norkunas norkunas force-pushed the serializer-timestamp-normalization-cast branch 2 times, most recently from dc03538 to b3d55df Compare December 20, 2023 06:40
@norkunas norkunas mentioned this pull request Dec 20, 2023
Closed
@norkunas norkunas force-pushed the serializer-timestamp-normalization-cast branch from b3d55df to bae7e35 Compare December 20, 2023 06:45
@norkunas norkunas requested a review from mtarld December 20, 2023 06:49
@norkunas
Copy link
Contributor Author

Should I skip those test cases on 32bit systems when value is > PHP_INT_MAX?

@derrabus
Copy link
Member

Should I skip those test cases on 32bit systems when value is > PHP_INT_MAX?

Either that or skip the int cast on those systems.

@norkunas norkunas force-pushed the serializer-timestamp-normalization-cast branch 5 times, most recently from 969e59b to 0c85461 Compare December 20, 2023 11:30
@norkunas norkunas changed the title [Serializer] Allow to cast certain DateTime formats to int/float [Serializer] Add DateTimeNormalizer::CAST_KEY context option Dec 20, 2023
@norkunas
Copy link
Contributor Author

norkunas commented Jan 5, 2024

Anybody to review?

mtarld
mtarld reviewed Jan 13, 2024
View reviewed changes
Copy link
Contributor

@mtarld mtarld left a comment

Choose a reason for hiding this comment

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

Code looks good to me. But I'm wondering that maybe we can add a check to make sure that the cast value is consistent with the format? So that it won't be possible to cast d/m/Y to an int for example. WDYT?

@norkunas
Copy link
Contributor Author

Code looks good to me. But I'm wondering that maybe we can add a check to make sure that the cast value is consistent with the format? So that it won't be possible to cast d/m/Y to an int for example. WDYT?

But then we would go back to format guessing, which derrabus wanted to avoid :)

@mtarld
Copy link
Contributor

mtarld commented Jan 13, 2024

Oh yeah, I missed that part... Thanks, LGTM then!

@fabpot fabpot force-pushed the serializer-timestamp-normalization-cast branch from 0c85461 to e5e925e Compare February 3, 2024 13:24
@fabpot
Copy link
Member

fabpot commented Feb 3, 2024

Thank you @norkunas.

@fabpot fabpot merged commit 6fd41bb into symfony:7.1 Feb 3, 2024
4 of 9 checks passed
@fabpot fabpot mentioned this pull request Feb 3, 2024
Closed
@norkunas norkunas deleted the serializer-timestamp-normalization-cast branch February 3, 2024 13:25
@SzymonKaminski SzymonKaminski mentioned this pull request Feb 3, 2024
Merged
OskarStark added a commit to symfony/symfony-docs that referenced this pull request Feb 4, 2024
@OskarStark
feature #19488 [Serializer] Document DateTimeNormalizer::CAST_KEY c…
a95c266 
…ontext option (SzymonKaminski)

This PR was merged into the 7.1 branch.

Discussion
----------

[Serializer] Document `DateTimeNormalizer::CAST_KEY` context option

PR documents new context option `CAST_KEY` of `DateTimeNormalizer` that allows to cast objects to integers or floats.

Resolves #19485
Feature PR: symfony/symfony#53056

Commits
-------

5495c1d [Serializer] Document DateTimeNormalizer::CAST_KEY context option
@fabpot fabpot mentioned this pull request May 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valtzu valtzu valtzu left review comments

@fabpot fabpot fabpot approved these changes

@mtarld mtarld mtarld approved these changes

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

@derrabus derrabus Awaiting requested review from derrabus

Assignees
No one assigned
Labels
Feature Serializer Status: Reviewed
Projects
None yet
Milestone
7.1
Development

Successfully merging this pull request may close these issues.

[Serializer] DateTimeNormalizer U or U.u format as int/float
6 participants
@norkunas @derrabus @mtarld @fabpot @valtzu @carsonbot