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

Patch 1 #30

Closed
wants to merge 3 commits into from
Closed

Patch 1 #30

wants to merge 3 commits into from

Conversation

ChristopherRabotin
Copy link
Contributor

Extended to more databases: sqlite, Postegres and MS-SQL (dblib). Note that I only tested MySQL and sqlite! If you get a PDO Exception in one of the queries, it's probably because of a syntax error for the specific driver. To change that, go to the bottom of the file and edit the TypeAdapter class. For the good of mankind, don't hesitate to request a pull from your fork.

Updated README.md , but that needs to be double checked since I've never used the Config module.

The diff looks huge, but that's mainly because of indentation change from my editor. Changes are minimal.

ChristopherRabotin added 3 commits October 9, 2013 18:45
…ions.

WARNING: Constructor signature has changed!
NOTE: Although PDO supports several database types, this update only includes sqlite in addition to MySQL.
…) Postgres and MS-SQL (dblib PDO driver).

Extended to more databases: sqlite, Postegres and MS-SQL (dblib). Note that I only tested MySQL and sqlite! If you get a PDO Exception in one of the queries, it's probably because of a syntax error for the specific driver. To change that, go to the bottom of the file and edit the TypeAdapter class. For the good of mankind, don't hesitate to request a pull from your fork.
I've never used Config::get and don't have that package on my current PHP installation. $dump->type may be able to be set from Config.
@clouddueling
Copy link
Contributor

I'm liking this idea but would you mind making this PHP-FIG and one commit would make me more comfortable in merging this.

@ChristopherRabotin
Copy link
Contributor Author

Not a problem, I'll do so. I also actually noticed some bugs in my updates after more thorough testing on a MySQL database.

@clouddueling
Copy link
Contributor

Idk if this is applicable but if you took your tests (without database obviously) and adding them to the commit just for easy testing. That would be so helpful!

@ChristopherRabotin
Copy link
Contributor Author

I guess I could try to create tom unit testing. My tests are actually manual in the sense that I realized something failed when exporting and then corrected the issue.

@clouddueling
Copy link
Contributor

I don't think unit testing is really necessary just a file that runs each of the methods with various options. Something easy that people can add to.

@ChristopherRabotin
Copy link
Contributor Author

That's a good idea. I'll do that.

@clouddueling
Copy link
Contributor

I just took this and refactored the class a bit, but you should see this code in the current version.
I need to write a style guide or grab one from somewhere.

@ChristopherRabotin
Copy link
Contributor Author

Glad this made it to the main branch! I was going to format the code for you today (since I was busy on another project during the weekend).

clxmstaab pushed a commit to staabm/mysqldump-php that referenced this pull request Feb 20, 2024
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.

None yet

2 participants