-
Notifications
You must be signed in to change notification settings - Fork 11
DBZ-8301 Support for decimal.handling.mode #29
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
Conversation
Hi @jpechane. Thank you for your valuable contribution. |
Welcome as a new contributor to Debezium, @jpechane. Reviewers, please add missing author name(s) and alias name(s) to the COPYRIGHT.txt and Aliases.txt respectively. |
@aziza-calm Would it be possible to add a test to verifythe behaviour? |
sorry for maybe silly question, but should I first clone your forked copy of repo, and add test there? |
No, Isuppose you have a fork repo of this one. So you should just add another class side- by-side with |
I do have a fork repo, and I did some changes on it before I noticed that my PR was closed. How to put those changes in new 2 PRs? |
You should be able to just cherry-pick the commits you need to apply to the new branches/PRs |
@@ -24,6 +26,10 @@ public class As400ValueConverters extends JdbcValueConverters { | |||
public As400ValueConverters() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we delete the decault constructor? It doesn't look like it's used anywhere else
Can I just check - does this doesn't change existing default behaviour or require changes to existing config? |
Hi @jpechane. Thank you for your valuable contribution. |
@msillence No, there is no change to the default behaviour. |
great, thanks, fine by me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
https://issues.redhat.com/browse/DBZ-8301