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

config: add configuration entry make TiDB version string configurable #13775

Merged
merged 38 commits into from
Dec 4, 2019
Merged

config: add configuration entry make TiDB version string configurable #13775

merged 38 commits into from
Dec 4, 2019

Conversation

reafans
Copy link
Contributor

@reafans reafans commented Nov 27, 2019

What problem does this PR solve?

It can improve the hard-coding of serverVersion property.

What is changed and how it works?

add the serverVersion in config/config.go and config/config.toml.example

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported variable/fields change

Release note

@codecov
Copy link

codecov bot commented Nov 27, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #13775   +/-   ##
===========================================
  Coverage   80.3003%   80.3003%           
===========================================
  Files           478        478           
  Lines        118784     118784           
===========================================
  Hits          95384      95384           
  Misses        15904      15904           
  Partials       7496       7496

Copy link
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

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

Please add some unit tests

@reafans reafans dismissed a stale review via 6b35446 November 27, 2019 12:57
tidb-server/main.go Outdated Show resolved Hide resolved
tidb-server/main.go Outdated Show resolved Hide resolved
@reafans
Copy link
Contributor Author

reafans commented Dec 4, 2019

/run-unit-test

Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@bb7133 bb7133 added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 4, 2019
@@ -74,6 +74,9 @@ split-region-max-num = 1000
# In order to support "drop primary key" operation , this flag must be true and the table does not have the pkIsHandle flag.
alter-primary-key = false

# serverVersion is used to show the version of tidb when log in.
Copy link
Member

Choose a reason for hiding this comment

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

Please add the comment to explain the version when server-version = ""(Original TiDB version)

@bb7133
Copy link
Member

bb7133 commented Dec 4, 2019

Notice that the result of select @@version is not affected, I'm not sure if this is good enough for this feature.

@bb7133 bb7133 added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 4, 2019
@bb7133
Copy link
Member

bb7133 commented Dec 4, 2019

/run-all-tests

@bb7133 bb7133 changed the title config:improve the hard-coding of serverVersion property. config: add configuration entry make TiDB version string configurable Dec 4, 2019
@bb7133 bb7133 merged commit d4d14db into pingcap:master Dec 4, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

cherry pick to release-2.1 failed

@sre-bot
Copy link
Contributor

sre-bot commented Dec 4, 2019

cherry pick to release-3.0 failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/config contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

config:improve the configurability of serverVersion property
4 participants