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

Compatability with new JMS\PropertyMetadata interface #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iVariable
Copy link

JMS\SerializerBundle\Metadata\PropertyMetadata interface for $type has changed.

Here is a quickfix for it.

@stof
Copy link

stof commented Nov 21, 2012

It would be even better if this bundle could support both JMSSerializerBundle 0.9 and 1.0.x-dev for now (some people prefer waiting for releases before updating)

@iVariable
Copy link
Author

I couldn't find right way to determine version of JMSSerializer. Only indirect, such as "if (is_array($type))" or "if ( interface_exists("\JMS\SerializerBundle\Serializer\Handler\SerializationHandlerInterface") )".

Do you think it's good enough to implement it that way?

@stof
Copy link

stof commented Nov 28, 2012

@iVariable you don't need to check the version IMO. Using is_array($type) is fine IMO. This is how NelmioApiDocBundle implemented it

@hardchor
Copy link

hardchor commented Dec 5, 2012

Any chance to amend and push this through?

JMS\SerializerBundle\Metadata\PropertyMetadata interface for type has changed.

Here is a quickfix for it.

Two versions of JMS Serializer support
@iVariable
Copy link
Author

Oops, sorry. Have missed previous comment.
@stof is it good enough now?

PS: I preferred to make simple "if" instead of something like getTypeFromOldSerializer and getTypeFromNewSerializer :))

@stof
Copy link

stof commented Dec 5, 2012

Well, instead of duplicating the whole code of the function (requiring to maintain it twice), it would be easier to normalize the type at the beggining:

if (is_array($type)) {
    // handle JMSSerializerBundle >= 0.10
    $type = $type['name'];
}

// All the existing code here without any modification

@iVariable
Copy link
Author

Don't you forgot about $type['params'] part?
Now "type" in JMS Metadata is parsed in some way and I don't think it's a good idea to denormalize it back to string. It's not only $type = $type['name']; but a bit more complicated code.

And as for me I think this patch is just a quickfix before maintaining JMSSerializer version 1.0. It'll be better to remove support of old Serializer, after Serializer version 1 release. Don't you think so?

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