-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Prepare for the new serialization mechanism #30965
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
Conversation
999800a
to
c4ce96e
Compare
This PR was merged into the 1.x branch. Discussion ---------- Prepare for the new serialization mechanism #eufossa related to symfony/symfony#30965 So we are future proof and it will prevent deprecations for symfony/symfony#30304. Almost future proof actually because the void return type cannot be added in 7.0. I guess we can see the minimum required PHP version here once 7.4 is out and see how we can handle compatibility 😕 Commits ------- 72d7abe Prepare for the new serialization mechanism
UPGRADE-4.3.md
Outdated
@@ -133,15 +133,15 @@ Security | |||
|
|||
After: | |||
```php | |||
protected function getState(): array | |||
protected function __serialize(): array |
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.
The methods are going to be public in PHP, so we should do the same here and probably mark the methods as @internal
for now (that's what I've done in the Mime component).
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.
actually, we cannot really make them internal when the class allows extensibility by inheritance, because that's the only way to add extra data to serialized payloads
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.
Just updated all the forgotten protected
to public
😅
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'm good with the change here, thanks.
c4ce96e
to
d412e77
Compare
Thank you @fancyweb. |
This PR was merged into the 4.3-dev branch. Discussion ---------- Prepare for the new serialization mechanism | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - #eufossa Should I maybe split this component by component ? https://wiki.php.net/rfc/custom_object_serialization has been accepted. Best viewed in "split" mode. This work is kind of required for #30304 so we don't trigger 30 deprecations from our own code base. Commits ------- d412e77 Prepare for the new serialization mechanism
…tion mechanism (fancyweb) This PR was merged into the 4.3-dev branch. Discussion ---------- [Security][TokenInterface] Prepare for the new serialization mechanism | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Continuation of #30965 Commits ------- e6455ea [Security][TokenInterface] Prepare for the new serialization mechanism
#eufossa
Should I maybe split this component by component ?
https://wiki.php.net/rfc/custom_object_serialization has been accepted.
Best viewed in "split" mode.
This work is kind of required for #30304 so we don't trigger 30 deprecations from our own code base.