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

DBZ-849 Refact read mysql sytem variables methods to make them more readable #597

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

artiship
Copy link
Contributor

@artiship artiship commented Aug 2, 2018

  1. Removed the unused param sql for the read system variables methods
  2. Extract the duplicate block from the two read system variables methods into a reusable method
  3. Extract the sql statement to be a field static final variable

@artiship artiship changed the title Refact read mysql sytewm variables methods to make them more readable DBZ-849 Refact read mysql sytem variables methods to make them more readable Aug 2, 2018
@gunnarmorling
Copy link
Member

Hey @artiship, thanks a lot! Sorry for the delay with reviewing, we're on summer holiday mode right now :) But I'll get back to this asap. In the meantime, could you reword the commit messages so they begin with the issue key ("DBZ-xyz ...")? Thanks!

@artiship
Copy link
Contributor Author

artiship commented Aug 9, 2018

@gunnarmorling Sorry forget the issue key. I'll figure out how to amend the commit message.

And these are three little changes, but they are not very related to each other. Should I make them into different PRs?

@artiship
Copy link
Contributor Author

artiship commented Aug 9, 2018

I've removed the other two commits into two separate PRs.

@jpechane jpechane merged commit bb4ecc7 into debezium:master Aug 14, 2018
@jpechane
Copy link
Contributor

Applied, thanks @artiship !

@gunnarmorling
Copy link
Member

Hey @artiship, could you give us your name so we can add it to the list of contributors? Thanks!

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