Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Serializer][PropertyInfo] Fix support for "false" built-in type on PHP 8.2#45981

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@alexandre-daubois
Copy link
Member

@alexandre-dauboisalexandre-daubois commentedApr 8, 2022•
edited
Loading

QA
Branch?4.4
Bug fix?yes (new language feature support)
New feature?no
Deprecations?no
TicketsNA
LicenseMIT
Doc PRNA

false andnull built-in types have been merged in PHP 8.2 a few hours ago:php/php-src#7546 🎉
RFC:https://wiki.php.net/rfc/null-false-standalone-types

PropertyInfo and Serializer both need an update to support them.

@carsonbotcarsonbot added this to the4.4 milestoneApr 8, 2022
@carsonbotcarsonbot changed the title[Serializer][PropertyInfo] Add support of false built-in type (from PHP 8.2)[PropertyInfo][Serializer] Add support of false built-in type (from PHP 8.2)Apr 8, 2022
@alexandre-dauboisalexandre-daubois changed the title[PropertyInfo][Serializer] Add support of false built-in type (from PHP 8.2)[PropertyInfo][Serializer] Add support of false built-in type (available from PHP 8.2)Apr 8, 2022
@alexandre-dauboisalexandre-dauboisforce-pushed thefix-false-builtin-type branch 2 times, most recently from9512543 tod915fe4CompareApril 9, 2022 13:25
@michaljusiega
Copy link
Contributor

Thanks for PR. Can we wait forhttps://wiki.php.net/rfc/true-type RFC also?

@alexandre-daubois
Copy link
MemberAuthor

I planned to do it as soon as it is merged if this PR is merged before "true" is available, but yeah I can definitely embed it as well in this pull request 👍

michaljusiega reacted with hooray emoji

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Works for me, with minor comments, thanks.
(true should wait until the RFC is approved)

alexandre-daubois and chapterjason reacted with thumbs up emoji
@chalasr
Copy link
Member

chalasr commentedApr 27, 2022•
edited
Loading

@alexandre-daubois Can you update the PropertyInfo's conflict rule + dev requirement for symfony/serializer so that it only allows serializer v4.4.42 or higher? That should fix the low-deps build

@alexandre-daubois
Copy link
MemberAuthor

I'll take a look into that as soon as possible!

@alexandre-dauboisalexandre-dauboisforce-pushed thefix-false-builtin-type branch 2 times, most recently fromf074bda to1e4f810CompareMay 9, 2022 18:36
@chalasr
Copy link
Member

Wait, it's the opposite actually 🤦‍♂️ Serializer needssymfony/property-info >=4.4.1. Sorry for the bad hint!

@alexandre-daubois
Copy link
MemberAuthor

Right, makes more sense 😄 At least, I figured out what are high-deps/low-deps builds. Thank you Robin!

@nicolas-grekasnicolas-grekas changed the title[PropertyInfo][Serializer] Add support of false built-in type (available from PHP 8.2)[Serializer][PropertyInfo] Fix support for "false" built-in type on PHP 8.2May 10, 2022
}

if (('is_'.$builtinType)($data)) {
if ('false' ===$builtinType ||('is_'.$builtinType)($data)) {

Choose a reason for hiding this comment

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

I've inlined the value of the constant: the minimum version bump is not worth it IMHO.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Alright, will do it as well forphp/php-src#8326 when it's accepted. Thanks 👍

/**
* @requires PHP 8.2
*/
publicfunctiontestFalseBuiltInTypes()

Choose a reason for hiding this comment

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

This test case was triggering a warning:This test did not perform any assertions

I fixed it.

@nicolas-grekas
Copy link
Member

Thank you@alexandre-daubois.

@nicolas-grekasnicolas-grekas merged commitfef98bb intosymfony:4.4May 10, 2022
@nicolas-grekas
Copy link
Member

Looks like something very close was already merge in 5.3 in#45154, so I backported the patch on 4.4 ind7ffaf5, on top of this one.

@fabpotfabpot mentioned this pull requestMay 14, 2022
This was referencedMay 27, 2022
nicolas-grekas added a commit that referenced this pull requestJun 20, 2022
…HP 8.2) (bobahvas, alexandre-daubois)This PR was merged into the 6.2 branch.Discussion----------[Serializer] Add support of true built-in type (from PHP 8.2)| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -RFC:https://wiki.php.net/rfc/true-typePull request:php/php-src#8326Same as this PR to add support of `false` and `null` types:#45981Commits-------2ec0453 [Serializer] Add support of true built-in type (from PHP 8.2)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@dunglasdunglasdunglas approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

6 participants

@alexandre-daubois@michaljusiega@chalasr@nicolas-grekas@dunglas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp