-
Notifications
You must be signed in to change notification settings - Fork 286
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
Refactor and tidy package #64
Conversation
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.
Could you break this PR into smaller ones? So that we can easily merge simple ones first.
@@ -39,8 +44,8 @@ func NewCDCSuite() *CDCSuite { | |||
if err != nil { | |||
panic(err.Error()) | |||
} | |||
// create a schema | |||
schema, err := NewSchema(jobs, false) | |||
// create a picker |
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.
we can rename picker
, but not schema
🙂️
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.
about this Schema
, I will update it in next pr.
I will take @suzaku 's advice, think of Schema
as a storage
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.
ok
Co-Authored-By: amyangfei <amyangfei@gmail.com>
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.
LGTM
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.
LGTM
What problem does this PR solve?
TxnMounter
totxn.Mounter
Check List
Tests