-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
UCP: Migrate dataForTiDBClusterInfo
from package infoschema
to executor
#15230
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15230 +/- ##
===========================================
Coverage ? 80.3288%
===========================================
Files ? 503
Lines ? 133063
Branches ? 0
===========================================
Hits ? 106888
Misses ? 17748
Partials ? 8427 |
@@ -309,3 +326,207 @@ func (s *testInfoschemaTableSuite) TestPartitionsTable(c *C) { | |||
|
|||
tk.MustExec("DROP TABLE `test_partitions`;") | |||
} | |||
|
|||
var _ = SerialSuites(&testInfoschemaClusterTableSuite{testInfoschemaTableSuite: &testInfoschemaTableSuite{}}) |
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.
Do we realy need to nest the testInfoschemaTableSuite
in the testInfoschemaClusterTableSuite
?
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.
unneccessaryly, just for reuse code like done in the tables_test.go before, it is ok to decouple.
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.
Rest LGTM
@yzwqf gofmt failed, please run |
I'm new to git and very depressed, how is it now... |
Hi, @yzwqf Seems you do some misoperations on git, maybe you can do the following instructions to pull it back on the right path.
|
@yzwqf please resolve the conflicts. |
Rest LGTM, Hi @yzwqf, thanks for your contribution. The failed check_dev shows that the code need format, just use |
"github.com/pingcap/tidb/statistics" | ||
"github.com/pingcap/tidb/util/testutil" | ||
"net" |
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 |
/merge |
Your auto merge job has been accepted, waiting for 14651 |
/run-all-tests |
@yzwqf merge failed. |
/run-all-tests |
What problem does this PR solve?
UCP #15031
What is changed and how it works?
Migrate dataForTiDBClusterInfo from package infoschema to executor.
Check List
Tests