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

plancodec: add test for plan id to avoid change the plan id. #17385

Merged
merged 12 commits into from
Jun 2, 2020

Conversation

crazycs520
Copy link
Contributor

Signed-off-by: crazycs520 crazycs520@gmail.com

What problem does this PR solve?

#15507 remove a plan that causes the rest plan ID was changed, It was unexpected.

  • Use constant value as plan id, remove iota
  • Add test for plan id.

What is changed and how it works?

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

  • No

Release note

  • No. (Add test for plan id to avoid change the plan id.)

Signed-off-by: crazycs520 <crazycs520@gmail.com>
Signed-off-by: crazycs520 <crazycs520@gmail.com>
@crazycs520 crazycs520 added type/enhancement The issue or PR belongs to an enhancement. sig/planner SIG: Planner labels May 25, 2020
@crazycs520 crazycs520 requested a review from a team as a code owner May 25, 2020 08:38
@ghost ghost requested review from eurekaka and removed request for a team May 25, 2020 08:38
util/plancodec/id.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented May 25, 2020

Codecov Report

Merging #17385 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #17385   +/-   ##
===========================================
  Coverage   79.4458%   79.4458%           
===========================================
  Files           520        520           
  Lines        139957     139957           
===========================================
  Hits         111190     111190           
  Misses        19798      19798           
  Partials       8969       8969           

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@crazycs520
Copy link
Contributor Author

/run-all-tests

Copy link
Contributor

@LittleFall LittleFall left a comment

Choose a reason for hiding this comment

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

lgtm

LittleFall
LittleFall previously approved these changes May 29, 2020
Copy link
Contributor

@LittleFall LittleFall left a comment

Choose a reason for hiding this comment

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

rest lgtm

util/plancodec/id_test.go Outdated Show resolved Hide resolved
Co-authored-by: Zhi Qi <30543181+LittleFall@users.noreply.github.com>
@LittleFall
Copy link
Contributor

lgtm

@crazycs520
Copy link
Contributor Author

/run-unit-test

@crazycs520 crazycs520 merged commit f030db5 into pingcap:master Jun 2, 2020
@crazycs520 crazycs520 deleted the plan-id branch June 2, 2020 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants