Skip to content

parser: better error message #133#8950

Merged
jackysp merged 6 commits intopingcap:masterfrom
lnhote:parser_better_errmsg
Jan 7, 2019
Merged

parser: better error message #133#8950
jackysp merged 6 commits intopingcap:masterfrom
lnhote:parser_better_errmsg

Conversation

@lnhote
Copy link
Contributor

@lnhote lnhote commented Jan 4, 2019

What problem does this PR solve?

Make parser error/warn message useful.
see pingcap/parser#86

What is changed and how it works?

  1. changed Errorf function, add more context.
  2. add/change testcases.
    see https://github.com/pingcap/parser/pull/133/files

Check List

Tests

  • Unit test

This change is Reviewable

@CLAassistant
Copy link

CLAassistant commented Jan 4, 2019

CLA assistant check
All committers have signed the CLA.

@zz-jason
Copy link
Member

zz-jason commented Jan 5, 2019

Looks like we should merge pingcap/parser#133 first, then use the latest "pingcap/parser".

@zz-jason zz-jason added contribution This PR is from a community contributor. component/parser type/enhancement The issue or PR belongs to an enhancement. labels Jan 5, 2019
@lysu
Copy link
Contributor

lysu commented Jan 7, 2019

/run-all-tests

@lysu
Copy link
Contributor

lysu commented Jan 7, 2019

LGTM

@lysu lysu added status/LGT1 Indicates that a PR has LGTM 1. status/all tests passed labels Jan 7, 2019
@lysu
Copy link
Contributor

lysu commented Jan 7, 2019

@tiancaiamao @jackysp PTAL

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@78a51a4). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #8950   +/-   ##
=========================================
  Coverage          ?   67.52%           
=========================================
  Files             ?      363           
  Lines             ?    75111           
  Branches          ?        0           
=========================================
  Hits              ?    50719           
  Misses            ?    19914           
  Partials          ?     4478

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78a51a4...d021275. Read the comment docs.

Copy link
Contributor

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp jackysp merged commit 39e1dfe into pingcap:master Jan 7, 2019
@breezewish
Copy link
Member

/run-integration-tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/parser contribution This PR is from a community contributor. status/LGT1 Indicates that a PR has LGTM 1. type/enhancement The issue or PR belongs to an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants