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 PdoReader #126

Merged
merged 6 commits into from
Jan 22, 2015
Merged

Add PdoReader #126

merged 6 commits into from
Jan 22, 2015

Conversation

rjmackay
Copy link
Contributor

Add PdoReader class and unit tests.
Roughly copied from DBAL reader. I needed a simple DB reader that doesn't require DBAL.

@rjmackay
Copy link
Contributor Author

rjmackay commented Dec 1, 2014

Looks like this fails on PHP 5.3.. I can remove the [array, syntax] stuff to array() if needed

@ddeboer
Copy link
Owner

ddeboer commented Dec 6, 2014

Thanks! Please make the code PHP 5.3 compatible (for now). And could you add a note to the docs, too?

@rjmackay
Copy link
Contributor Author

rjmackay commented Dec 8, 2014

Updated.. but I have no idea why the tests are failing.. they pass fine for me locally.


// Build schema
$connection->query('CREATE TABLE pdo_group (
id INTEGER PRIMARY KEY,
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you set this column to autoincrement to pass the tests?

@Baachi
Copy link
Collaborator

Baachi commented Jan 19, 2015

@rjmackay Are you still working on it?

@rjmackay
Copy link
Contributor Author

I've had to shelve it while I continue on other work.
I'll try to come back to it soon and fix the tests.

Conflicts:
	tests/Ddeboer/DataImport/Tests/Reader/PdoReaderTest.php
@rjmackay
Copy link
Contributor Author

Excellent finally got a test pass. @Baachi any other changes needed?

@ddeboer
Copy link
Owner

ddeboer commented Jan 20, 2015

Looks good! Can you add a short entry for this reader to the docs (README.md)?

@ddeboer ddeboer mentioned this pull request Jan 22, 2015
ddeboer added a commit that referenced this pull request Jan 22, 2015
@ddeboer ddeboer merged commit c06703d into ddeboer:master Jan 22, 2015
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