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

check if the data implements SerializationHandlerInterface and in that ca #23

Merged
merged 14 commits into from
Sep 5, 2011
Merged

check if the data implements SerializationHandlerInterface and in that ca #23

merged 14 commits into from
Sep 5, 2011

Conversation

lsmith77
Copy link
Contributor

@lsmith77 lsmith77 commented Sep 3, 2011

check if the data implements SerializationHandlerInterface and in that case call it before going through the custom handlers (guess a similar change would need to be made for deserialization)

addresses #21

@schmittjoh
Copy link
Owner

Could you move this to the GraphNavigator.php (this avoids code duplication)? Also, it should have a small test.

@lsmith77
Copy link
Contributor Author

lsmith77 commented Sep 4, 2011

ok .. will have a look.

…t case call it before going through the custom handlers (guess a similar change would need to be made for deserialization)
@lsmith77
Copy link
Contributor Author

lsmith77 commented Sep 4, 2011

done

@lsmith77
Copy link
Contributor Author

lsmith77 commented Sep 4, 2011

hmm i fear that with this change deserialization might be broken .. if i read it properly GraphNavigator.php is used in both cases? does $isSerialization tell me inside the accept() method if i am running in serialization or deserialization mode?

@lsmith77
Copy link
Contributor Author

lsmith77 commented Sep 4, 2011

i have merged your changes and fixed the code accordingly .. the tests pass now.

@schmittjoh
Copy link
Owner

Are you sure you committed everything? I got some failures here.

@lsmith77
Copy link
Contributor Author

lsmith77 commented Sep 5, 2011

ooppss ... ran the tests so often that i forgot that i had a filter set .. will have a look.

@lsmith77
Copy link
Contributor Author

lsmith77 commented Sep 5, 2011

done

@schmittjoh schmittjoh merged commit 923e572 into schmittjoh:master Sep 5, 2011
@schmittjoh
Copy link
Owner

I've turned this off by default as it is mainly to ease migration, but otherwise merged everything.

@lsmith77
Copy link
Contributor Author

lsmith77 commented Sep 5, 2011

Hmm .. I dont think its mainly for migration. I think this is an important feature in case someone has a wide range of classes that need to be serialized with custom logic that is simply not feasible to do in a single handler and that would otherwise lead to too many custom handlers. but its easy enough to enable for those users, so its all good i guess :)

chasewoo pushed a commit to royuan/JMSSerializerBundle that referenced this pull request Aug 7, 2013
Fix DateTimeZone issue when using the DateTime type
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.

2 participants