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

Add toArray and fromArray methods to the serializer #435

Merged
merged 3 commits into from
May 1, 2015

Conversation

tystr
Copy link
Contributor

@tystr tystr commented Apr 22, 2015

This PR implements the solution suggested by @schmittjoh to add additional methods for dealing with array input/output instead of adding a new format for arrays.

#20 (comment)

TODO

  • Add some test cases

@tystr tystr force-pushed the add-array-methods branch from 817241d to bd3f7d6 Compare April 22, 2015 01:23
@tystr tystr changed the title WIP - Add toArray and fromArray methods to the serializer Add toArray and fromArray methods to the serializer Apr 22, 2015
@@ -0,0 +1,1307 @@
<?php
Copy link
Owner

Choose a reason for hiding this comment

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

Let's not replicate the entire test suite. One simple test per method should be ok.

@tystr
Copy link
Contributor Author

tystr commented Apr 27, 2015

I've just returned from vacation. I'll wrap this up this week.

@davidwindell
Copy link

👍 thanks for your time on this @tystr

@tystr tystr force-pushed the add-array-methods branch from 4593154 to 8291086 Compare April 28, 2015 15:46
@tystr
Copy link
Contributor Author

tystr commented Apr 28, 2015

@schmittjoh I've simplified the tests for the array methods down to a couple of very rudimentary tests and added handling for ArrayObject.

$visitor->setNavigator($this->navigator);
$this->navigator->accept($visitor->prepare($data), null, $context);

return (array) $visitor->getRoot();
Copy link
Owner

Choose a reason for hiding this comment

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

We should do this for the entire array (including children) as these might have an array object, too. Could you also add this to the test-suite?

@rdohms
Copy link

rdohms commented Apr 29, 2015

👍

@rdohms
Copy link

rdohms commented May 1, 2015

Where are we with this? Any ideas when a release with it will come out?

@schmittjoh schmittjoh merged commit b3ea965 into schmittjoh:master May 1, 2015
@schmittjoh
Copy link
Owner

I've made a few tweaks, and merged this. We'll need to give this a bit of time in master before thinking about a release.

@davidwindell
Copy link

Great, thanks @tystr and @schmittjoh for getting this done!

@davidwindell
Copy link

Just a quick update, this has been used by our app in production since 1 May and all good so far!

@rdohms
Copy link

rdohms commented Jun 22, 2015

can we get this in a release?

@rdohms
Copy link

rdohms commented Jul 3, 2015

Aha, you did, v1.0.0 now supports this. 👍

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.

4 participants