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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Feature/generator #75

wants to merge 7 commits into
base: master


None yet
1 participant
Copy link

commented Feb 25, 2018

Please note this is just a WIP for issue #68 .

I adjusted the Reader interface to handle Generators. I updated the ArrayReader and AppendReader to follow the interface. I didn't find a way to keep BC and use Generators behind the scenes 馃槩 . If you have an idea, let me know.

I also bumped the php version to 7.0 to handle generators efficiently. I don't this should be to problematic
for most of the users.

I also removed the CountableIteratorReader and the IteratorReader because the new ArrayReader now handles Traversable objects as well and the new interface brokes BC anyway. So i think we can safely remove this classes.

So i added a which document what has been changed and how to migrate.

If this PR gets accepted, i will send PR to the other readers as well.

  • Update Reader interface
  • Update ArrayReader
  • Update AppendReader
  • Update OneToManyIterator
  • Add Changelog
  • Update documentation
  • Update Pdo Reader
  • Update Csv Reader
  • Update Dbal Reader
  • Update Excel Reader
  • Update Spout Reader

@Baachi Baachi requested review from sagikazarmark and ddeboer Feb 25, 2018

$cnt = 0;
foreach ($this->readers as $reader) {

This comment has been minimized.

Copy link

Baachi Feb 25, 2018

Author Contributor

Not sure if we should use the nikc/iter library for such case? I try to avoid adding new libraries to make the migration path as easy as possible.


This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2018

1 similar comment

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.