-
-
Notifications
You must be signed in to change notification settings - Fork 949
Render BCMath\Number (PHP 8.4+) as string instead of object #7555
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
base: 4.2
Are you sure you want to change the base?
Conversation
a5e0533 to
0f95799
Compare
|
interesting, I'm wondering if this couldn't be added to 4.2 though. |
| ]; | ||
| } | ||
|
|
||
| if (class_exists(\BcMath\Number::class) && is_a($className, \BcMath\Number::class, true)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (class_exists(\BcMath\Number::class) && is_a($className, \BcMath\Number::class, true)) { | |
| if (is_a($className, \BcMath\Number::class, true)) { |
IIRC, the call to is_a() will return false if the class doesn't exist.
| // symfony/serializer:7.3 added the NumberNormalizer | ||
| // symfony/framework-bundle:7.3 added the serializer.normalizer.number` service | ||
| // if symfony/serializer >= 7.3 and symfony/framework-bundle < 7.3, the service is registred | ||
| if (class_exists(NumberNormalizer::class) && !$container->has('serializer.normalizer.number')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to just bump the minimum required versions to avoid supporting this edge case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we just bumped everything to 7.4 if I'm not mistaken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mhh we did not but indeed we should bump this to 7.4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t see the bump to 7.4 on the branch 4.2, I was wondering which components should I bump ?
https://github.com/api-platform/core/blob/4.2/src/JsonSchema/composer.json#L31
https://github.com/api-platform/core/blob/4.2/src/Serializer/composer.json#L30
https://github.com/api-platform/core/blob/4.2/src/Symfony/composer.json#L46
And there’s currently no dependency on symfony/framework-bundle for the api-platform/symfony, should I add it ?
ef978e3 to
e0a4812
Compare
e0a4812 to
0320ed9
Compare
0320ed9 to
b0c4939
Compare
Render BCMath\Number (PHP 8.4+) as string instead of object
Before
After
The
symfony/serializer:7.3added theNumberNormalizer, thesymfony/framework-bundle:7.3registered it.In CI and the job PHPUnit (PHP 8.4),
api-platforminstallsymfony/framework-bundle:7.2if I try locally