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

ddl: add missing ErrorCount++ when update Error #15373

Merged
merged 7 commits into from
Mar 19, 2020

Conversation

zhiqiangxu
Copy link
Contributor

Just a missing bit of code.

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Mar 14, 2020
@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Mar 14, 2020
@zhiqiangxu zhiqiangxu closed this Mar 14, 2020
@zhiqiangxu zhiqiangxu reopened this Mar 14, 2020
@zz-jason zz-jason requested a review from zimulala March 14, 2020 15:43
@Deardrops
Copy link
Contributor

@zhiqiangxu thanks for your contributing, could you add a test case in this PR?

@zhiqiangxu
Copy link
Contributor Author

@zhiqiangxu thanks for your contributing, could you add a test case in this PR?

It seems there's no test for ErrorCount yet, no idea how to test it -_-b

@Deardrops
Copy link
Contributor

@zhiqiangxu zhiqiangxu#1 PTAL

@zhiqiangxu
Copy link
Contributor Author

@zhiqiangxu zhiqiangxu#1 PTAL

wow, that's cool !

@Deardrops
Copy link
Contributor

Here are some problem with run test cases parallel, I will take a look.

@Deardrops
Copy link
Contributor

/run-all-tests

Copy link
Contributor

@Deardrops Deardrops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, could you add some PR descriptions? Refer to TiDB's PR template

@zimulala zimulala added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 19, 2020
Copy link
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zimulala zimulala added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 19, 2020
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for your contribution

@AilinKid AilinKid added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Mar 19, 2020
@codecov
Copy link

codecov bot commented Mar 19, 2020

Codecov Report

Merging #15373 into master will not change coverage by %.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #15373   +/-   ##
===========================================
  Coverage   80.4152%   80.4152%           
===========================================
  Files           502        502           
  Lines        134028     134028           
===========================================
  Hits         107779     107779           
  Misses        17801      17801           
  Partials       8448       8448           

@AilinKid
Copy link
Contributor

/run-all-tests

@AilinKid
Copy link
Contributor

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 19, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 19, 2020

/run-all-tests

@sre-bot sre-bot merged commit 64ea0ea into pingcap:master Mar 19, 2020
@you06 you06 added this to the v4.0.0-rc milestone Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor. sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants