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.

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

[Serializer] Deprecate SerializerAwareEncoder #18483

Merged
merged 1 commit into from Jun 8, 2016

Conversation

JhonnyL
Copy link
Contributor

@JhonnyL JhonnyL commented Apr 8, 2016

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets
License MIT
Doc PR

@@ -21,8 +23,10 @@
* @author Fabian Vogler <fabian@equivalence.ch>
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class XmlEncoder extends SerializerAwareEncoder implements EncoderInterface, DecoderInterface, NormalizationAwareInterface
class XmlEncoder implements EncoderInterface, DecoderInterface, NormalizationAwareInterface, SerializerAwareInterface
Copy link
Member

Choose a reason for hiding this comment

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

This is a BC break (in case someone type-hinted with SerializerAwareEncoder).

@JhonnyL
Copy link
Contributor Author

JhonnyL commented Apr 21, 2016

@xabbuh I reverted the changes to XmlEncoder.


/**
* SerializerAware Encoder implementation.
*
* @author Jordi Boggiano <j.boggiano@seld.be>
*
* @deprecated since version 3.1, to be removed in 4.0. Use the SerializerAwareTrait instead.
Copy link
Member

Choose a reason for hiding this comment

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

the deprecation warning must be triggered too

Copy link
Member

Choose a reason for hiding this comment

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

btw, this will be 3.2, not 3.1, as we are already feature-frozen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that would trigger a deprecation warning whenever XmlEncoder is used, with no option to fix it.

I will update to 3.2

Copy link
Member

Choose a reason for hiding this comment

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

@stof Isn't the docblock annotation enough as the rest is handled by the debug class loader?

@JhonnyL JhonnyL force-pushed the deprecate-SerializerAwareEncoder branch from 67c4c3b to af57467 Compare April 21, 2016 08:20
@GuilhemN
Copy link
Contributor

GuilhemN commented Jun 7, 2016

LGTM 👍

@xabbuh
Copy link
Member

xabbuh commented Jun 8, 2016

👍

@fabpot
Copy link
Member

fabpot commented Jun 8, 2016

Thank you @JhonnyL.

@fabpot fabpot merged commit af57467 into symfony:master Jun 8, 2016
fabpot added a commit that referenced this pull request Jun 8, 2016
This PR was merged into the 3.2-dev branch.

Discussion
----------

[Serializer] Deprecate SerializerAwareEncoder

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Commits
-------

af57467 [Serializer] Deprecate SerializerAwareEncoder
@fabpot fabpot mentioned this pull request Oct 27, 2016
sandergo90 pushed a commit to sandergo90/symfony that referenced this pull request Jul 4, 2019
…JhonnyL)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[Serializer] Deprecate SerializerAwareEncoder

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Commits
-------

af57467 [Serializer] Deprecate SerializerAwareEncoder
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.

None yet

7 participants