8000 Prepare for the new serialization mechanism by fancyweb · Pull Request #30965 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 1 commit into from
Apr 8, 2019

Conversation

fancyweb
Copy link
Contributor
@fancyweb fancyweb commented Apr 7, 2019
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.

@fancyweb fancyweb force-pushed the new-serialization-mechanism branch 2 times, most recently from 999800a to c4ce96e Compare April 7, 2019 12:18
fabpot added a commit to twigphp/Twig that referenced this pull request Apr 7, 2019
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
Copy link
Member

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).

Copy link
Member

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

Copy link
Contributor Author

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 😅

Copy link
Member
@nicolas-grekas nicolas-grekas left a 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.

@fancyweb fancyweb force-pushed the new-serialization-mechanism branch from c4ce96e to d412e77 Compare April 7, 2019 18:20
@fabpot
Copy link
Member
fabpot commented Apr 8, 2019

Thank you @fancyweb.

@fabpot fabpot merged commit d412e77 into symfony:master Apr 8, 2019
fabpot added a commit that referenced this pull request Apr 8, 2019
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
chalasr pushed a commit that referenced this pull request Apr 10, 2019
…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
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@fancyweb fancyweb deleted the new-serialization-mechanism branch August 9, 2019 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0