Skip to content

[Serializer] Harden the ObjectNormalizer #17959

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

Closed
wants to merge 2 commits into from

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Feb 29, 2016

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

Transform \TypeErrors to catchable serializer exceptions.

Follows #17738 and completes #17660.

@@ -21,7 +21,7 @@
"require-dev": {
"symfony/yaml": "~2.8|~3.0",
"symfony/config": "~2.8|~3.0",
"symfony/property-access": "~2.8|~3.0",
"symfony/property-access": "~3.1",
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, this is "just" to make the test pass, right? The code change itself does not need that. So, IMO, we should keep the constraint as is and tweak the test instead.

@dunglas
Copy link
Member Author

dunglas commented Mar 4, 2016

Status: needs work

@dunglas dunglas force-pushed the srlzr_catch_typeerror branch from fc84692 to 5d4d0a6 Compare March 25, 2016 21:31
@dunglas
Copy link
Member Author

dunglas commented Mar 25, 2016

Status: needs review

@xabbuh
Copy link
Member

xabbuh commented Mar 26, 2016

You have update the minimum version required for the PropertyAccess component in the require-dev section to be ~3.1.

Status: Needs work

@dunglas
Copy link
Member Author

dunglas commented Mar 27, 2016

@xabbuh it should work with any supported PropertyAccess version as #18210 has been merged in 2.3. Maybe should I add a conflict section however.

@dunglas
Copy link
Member Author

dunglas commented Mar 27, 2016

Travis and AppVeyor not related.

Ping @symfony/deciders

@@ -27,6 +27,9 @@
"doctrine/annotations": "~1.0",
"doctrine/cache": "~1.0"
},
"conflict": {
"symfony/property-access": ">=3.0,<3.0.4|>=2.8,<2.8.4"
Copy link
Member

Choose a reason for hiding this comment

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

Then I would rather update the constraint in require-devto ~2.8,>=2.8.4|~3.0,>=3.0.4.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? The conflict line has the same effect and works in prod too.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Apr 17, 2016

👍 for 3.1. The composer.json update is fine to me.

@dunglas dunglas closed this in 3a165e5 Apr 18, 2016
@dunglas dunglas deleted the srlzr_catch_typeerror branch April 18, 2016 17:38
@fabpot fabpot mentioned this pull request May 13, 2016
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