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.

Sign up for GitHub

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

Jump to bottom

*: using standard error to replace terror (#19425) #19888

Merged
merged 5 commits into from
Sep 9, 2020
Merged

*: using standard error to replace terror (#19425) #19888

merged 5 commits into from
Sep 9, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #19425 to release-4.0

What problem does this PR solve?

Problem Summary: Replace error in tidb with standard error. parser pr is: pingcap/parser#982

What is changed and how it works?

Proposal: Standardize error codes and messages

What's Changed:
Change error message in test cases.

How it Works:
Error message changed after terror.Error has been replaced with errors.Error

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

  • Breaking backward compatibility

Release note

  • replace error code and message with standard error.

@ti-srebot
cherry pick pingcap#19425 to release-4.0
78e3e22 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot mentioned this pull request Sep 8, 2020
Merged
@ti-srebot ti-srebot added this to the v4.0.6 milestone Sep 8, 2020
@ti-srebot
Copy link
Contributor Author

@imtbkcat please accept the invitation then you can push to the cherry-pick pull requests.
https://github.com/ti-srebot/tidb/invitations

@imtbkcat
Copy link

imtbkcat commented Sep 8, 2020

parser pr: pingcap/parser#1014

bobotu
bobotu reviewed Sep 8, 2020
View reviewed changes
Copy link
Contributor

@bobotu bobotu left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor Author

@bobotu,Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments.See the corresponding SIG page for more information. Related SIGs: execution(slack),ddl(slack),planner(slack).

zimulala
zimulala reviewed Sep 8, 2020
View reviewed changes
go.mod Outdated
@@ -63,3 +63,5 @@ require (
)

go 1.13

replace github.com/pingcap/parser => github.com/imtbkcat/parser v0.0.0-20200908120703-5fc1c5049015
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update this line.

Copy link

Choose a reason for hiding this comment

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

fixed, PTAL @zimulala

@lysu
Copy link
Contributor

lysu commented Sep 8, 2020

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 8, 2020
AilinKid
AilinKid approved these changes Sep 9, 2020
View reviewed changes
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

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 9, 2020
@AilinKid
Copy link
Contributor

AilinKid commented Sep 9, 2020

/run-all-tests

@jackysp jackysp merged commit cf00089 into pingcap:release-4.0 Sep 9, 2020
imtbkcat pushed a commit that referenced this pull request Sep 11, 2020
Revert "*: using standard error to replace terror (#19425) (#19888)"
4ce2412 
This reverts commit cf00089.
@imtbkcat imtbkcat mentioned this pull request Sep 11, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zimulala zimulala zimulala left review comments

@imtbkcat imtbkcat imtbkcat left review comments

@bobotu bobotu bobotu left review comments

@AilinKid AilinKid AilinKid approved these changes

@bb7133 bb7133 Awaiting requested review from bb7133

@lonng lonng Awaiting requested review from lonng

@lysu lysu Awaiting requested review from lysu

@zz-jason zz-jason Awaiting requested review from zz-jason

Assignees

@imtbkcat imtbkcat

Labels
component/expression priority/release-blocker This PR blocks a release. Please review it ASAP. sig/execution SIG execution sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra status/LGT2 Indicates that a PR has LGTM 2. type/enhancement type/4.0-cherry-pick
Projects
None yet
Milestone
v4.0.6
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@ti-srebot @imtbkcat @lysu @AilinKid @zimulala @bobotu @jackysp