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 basic tests, vndr and travis #3

Merged
merged 1 commit into from
May 2, 2017
Merged

Add basic tests, vndr and travis #3

merged 1 commit into from
May 2, 2017

Conversation

SantoDE
Copy link

@SantoDE SantoDE commented Apr 24, 2017

This PR adds some basic tests, travis support and vndr for the dependency :)

Feedback please ;-)

@JamesStewy
Copy link
Owner

In dump.go, what is the purpose of the new print statement? It should probably be up to the library user to decide how errors are displayed (either printed, logged, etc.).

The tests are great. One small thing, near the bottom of the test file there is a variable called expected which contains the mysqldump version (currently 0.1.1). Could you replace the version with the constant version so it doesn't have to be updated.

Finally for vendoring, I personally don't like vendoring directly into the repo and thus use glide for vendoring in my other projects. If you like I can set that up as well as travis.

@SantoDE
Copy link
Author

SantoDE commented May 1, 2017

Hey @JamesStewy ,

sorry, I had some stuff to do ;) Thanks for your feedback. I removed the vendoring, so you're free to set it up like you wish :-)

Regards,
SantoDE

@JamesStewy JamesStewy merged commit a49ae48 into JamesStewy:master May 2, 2017
@JamesStewy
Copy link
Owner

Thanks again @SantoDE :).

@SantoDE SantoDE deleted the feature/tests-vndr branch May 3, 2017 08:52
BrandonRoehl added a commit to BrandonRoehl/go-mysqldump that referenced this pull request Sep 24, 2019
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.

2 participants