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 support for --replace option (offering to help) #171

Open
martinjoiner opened this issue Sep 2, 2019 · 6 comments
Open

Add support for --replace option (offering to help) #171

martinjoiner opened this issue Sep 2, 2019 · 6 comments

Comments

@martinjoiner
Copy link

It's a niche option that not many people use so I understand why it's not currently implemented but the --replace option that writes "REPLACE INTO...." instead of "INSERT INTO..." would have been handy for the project I am working on right now. (I had to do a find-and-replace on the generated dump file which feels a bit hacky)

Here's the MySQL doc page: https://dev.mysql.com/doc/refman/8.0/en/mysqldump.html#option_mysqldump_replace

I am happy to make a pull request with changes to code + documentation + tests if the maintainer approves.

@ifsnop
Copy link
Owner

ifsnop commented Sep 2, 2019 via email

@martinjoiner
Copy link
Author

Cool. I have started work on this. Can I just ask, what is the reason for all the class definitions being in 1 file? I see this needs to remain compatible with PHP 5.3 which is slightly unfamiliar to me given that I've been working in PHP 7.2 for the past few years. I will be revising my knowledge of the limitations of the older versions.

@ifsnop
Copy link
Owner

ifsnop commented Sep 3, 2019 via email

@martinjoiner
Copy link
Author

Of course! I expected that there would be a very good reason for keeping them all in a single file.

I am breaking the code that creates the statements into smaller methods, making the code more modulated so that little chunks of functionality can be unit tested individually for specific expectations.

For example, with the particular feature that I am adding we only need to know that the insert statements use the word "REPLACE" instead of "INSERT" when the option is truthy, we don't need to re-test every character of the whole dump file again as you already have lots of other good tests in place to check that stuff.

I'll probably finish the work over the weekend and send a PR next week.

@martinjoiner
Copy link
Author

Can I ask how you run the PHPUnit tests locally when developing? I'm having trouble checking my work by running the existing tests. I see that the Travis config looks super neat and it just rolls through loads of boxes running different versions of PHP, running the full test suite in each box. Is there an easy way of running the suite in just 1 version of PHP locally or should I build a VM?

@ifsnop
Copy link
Owner

ifsnop commented Oct 7, 2019 via email

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

No branches or pull requests

2 participants