-
Notifications
You must be signed in to change notification settings - Fork 72
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
299 mediumint unsigned fixed #300
Conversation
Can you create a PR with only the fix for mediumint? |
b4b6691
to
3f7118f
Compare
Done! |
Thanks for the PR. Is it possible for you to add a test here? https://github.com/Shopify/ghostferry/blob/master/test/integration/types_test.rb |
I can't get the ruby test thing to work on my machine, so I'm afraid I can't |
|
||
const MaxUint24 = 1<<24 - 1 | ||
|
||
func NewUint24(val int32) *Uint24 { |
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.
Is this needed? Can we not just use https://github.com/go-mysql-org/go-mysql/blob/master/mysql/parse_binary.go#L22-L31
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.
It would take a int32 to byte conversion, which in itself would require some code which you might not want to put in that location as well
Do you have an error? For ruby testing, you can use |
|
there seems to be more issues, will look further tomorrow |
you need mysql client development headers installed on your computer to install mysql2.
Having a comprehensive integration test may help you iron out all the issue in one go. |
fixes #299