Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle][Serializer] Move SerializerPass to the Serializer#21293
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
88a93f4 toa328e75Compare| "symfony/validator":"~3.2", | ||
| "symfony/yaml":"~3.2", | ||
| "symfony/property-info":"~2.8|~3.0", | ||
| "symfony/property-info":"~3.1", |
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.
Why this change?
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.
Because we change the framework's serializer requirement to 3.3, and the serializer in 3.3 requires this. The build with low deps was failing otherwise:
- Symfony\Bundle\FrameworkBundle\Tests\Functional\SerializerTest::testDeserializeArrayOfObject
Error: Class 'phpDocumentor\Reflection\FileReflector' not found
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.
Well, then it means that the serializer component should have the proper requirement or the proper conflict in place
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.
It should be a conflict because the Serializer doesn't require PropertyInfo.
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.
See#21309
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'll open a PR for that.
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.
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.
reverted thanks to#21309
a328e75 to9d3d592Compare…(chalasr)This PR was merged into the 3.1 branch.Discussion----------[Serializer] Add missing conflict for property-info<3.1| Q | A| ------------- | ---| Branch? | 3.1| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#21293 (comment)| License | MIT| Doc PR | n/aCommits-------60a0c4b [Serializer] Add missing conflict for property-info<3.1
…(chalasr)This PR was merged into the 3.1 branch.Discussion----------[Serializer] Add missing conflict for property-info<3.1| Q | A| ------------- | ---| Branch? | 3.1| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |symfony/symfony#21293 (comment)| License | MIT| Doc PR | n/aCommits-------60a0c4b [Serializer] Add missing conflict for property-info<3.1
c6169c3 tofea6cd4Comparechalasr commentedJan 24, 2017
@nicolas-grekas Is there any reason for adding this to the 3.x milestone but#21375 to the 3.3 one? They are about fixing the same ticket so if there is something wrong to you here, please tell me :) |
fea6cd4 toe7e5ac9Comparechalasr commentedJan 24, 2017
Updated this to make the serializer id and encoder/normalizer tags configurable through the constructor, defaulting to the ones used in the framework. |
e7e5ac9 to5e54135Compare5e54135 to95cf508Comparechalasr commentedJan 26, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
But, isn't a BC break to add a parent class with a new constructor to the deprecated pass?
I guess it's fine here. |
chalasr commentedFeb 7, 2017
Do we want to move forward on this for 3.3? It would be nice to have a thought, to know if it's worth working on other relevant passes listed in the ticket using the same approach. |
xabbuh commentedFeb 8, 2017
IMO it makes sense to move them. |
dunglas commentedFeb 8, 2017
👍 |
fabpot commentedFeb 16, 2017
Thank you@chalasr. |
…he Serializer (chalasr)This PR was merged into the 3.3-dev branch.Discussion----------[FrameworkBundle][Serializer] Move SerializerPass to the Serializer| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets | n/a| License | MIT| Doc PR | n/aPart of#21284Commits-------95cf508 [FrameworkBundle][Serializer] Move SerializerPass to the Serializer
Part of#21284