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

Support for changing default policy #427

Closed
wants to merge 3 commits into from
Closed

Conversation

plazmdk
Copy link

@plazmdk plazmdk commented Nov 23, 2014

For use with FOS rest bundle supporting defensive programming not exposing everything in services.

@GuilhemN
Copy link

GuilhemN commented Feb 3, 2016

👎 a third party bundle can't define which exclusion policy to use by default so this would create bugs.

@goetas
Copy link
Collaborator

goetas commented Dec 19, 2016

@GuilhemN can you elaborate a bit more? i was thinking to include schmittjoh/serializer#683 sooner of later...

@GuilhemN
Copy link

@goetas if you use a bundle that defines entities and depends on the jms serializer, it won't know the default policy and its entities won't be serialized as expected.
At least if you merge this, there should a warning about possible conflicts with third party bundles.

@goetas
Copy link
Collaborator

goetas commented Dec 19, 2016

but isn't an user choice the default exclusion strategy?

are you worried that the user decides to use a too strict exclusion strategy that can create unexpected behaviors with other bundles that acts as plugins with the serializer?

@GuilhemN
Copy link

@goetas yes, that's it. Moreover these unexpected behaviors won't be easy to catch.

@goetas
Copy link
Collaborator

goetas commented Dec 19, 2016

I see your point.

not sure what to do here then 😕 since i see also some advantages in it

@GuilhemN
Copy link

Maybe you can allow to configure this for each bundle, or in each file?

@goetas
Copy link
Collaborator

goetas commented Dec 19, 2016

currently this is a "per project feature" so can be enabled per any variation of the config.yml in symfony.

or in each file?

if you mean "per entity", this is already possible

per bundle, looks a bit weird.. if you are thinking something as https://symfony.com/doc/current/reference/configuration/assetic.html (per bundle enabler, i find is really weird in this case)

@GuilhemN
Copy link

GuilhemN commented Dec 19, 2016

I was thinking about the yaml configuration files, it could be possible to add something on top of it. But I don't know if that's worth it, and if it won't look hacky.

Otherwise, as you proposed, you can configure this at the project level with a proper warning but it looks like a technical debt to me as it might break at any time.

@goetas
Copy link
Collaborator

goetas commented Dec 30, 2016

Closing, merging this will lead to unexpected behaviors since will be possible to change defaults at runtime. Third party libraries will not have a consistent default.

thanks @GuilhemN for the hint :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants