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

Implementation of database drivers (recreated) #10

Open
wants to merge 53 commits into
base: master
Choose a base branch
from

Conversation

writ3it
Copy link

@writ3it writ3it commented Sep 12, 2019

I prepared version of Zebra_Mptt library with database drivers which makes possible to change a connection type to (f.e.) pdo. Zebra_mptt is very useful but in some projects is necessary to use pdo. Hardcoded mysqli dependency is a problem in legacy code or micro-frameworks based on PDO. In my opinion, database drivers are a good solution.

I reviewed Zebra_database for some implementations but I didn't found something like that. Proposed solution is compatible (through some adapter) with that library, I think.

Another advantage of drivers is making code testable. I implemented some tests.

I think that I discovered bug at copy function (using php 7.3). When node has children, loop (iterating over sources) is infinity because reference variable ($properties) is modified. I didn't spent too much time to investigate this bug. Maybe I am wrong or my php version is buggy.

Best regards and thanks for the library!

p.s. Now I see that i attached self to composer.json as contributor :) If it's problem I can removed it.
p.s.2. It's possible that I will develop the next version of my idea so, I created a branch which is in compatibility with main solution and the PR was re-created.

@tsmgeek
Copy link

tsmgeek commented Nov 1, 2021

Seems a good idea to allow for PDO. What are the sticking points to using this and/or merging it into mainline?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants