-
Notifications
You must be signed in to change notification settings - Fork 329
treat version as string instead of float when compare #2656
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
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @ghoshh! It looks like this is your first PR to pingcap/tiup 🎉 |
pkg/cluster/operation/check.go
Outdated
|
|
||
| // compareVersion compares two version strings v1 and v2. | ||
| // It returns 1 if v1 > v2, -1 if v1 < v2, and 0 if v1 == v2. | ||
| func CompareVersion(v1, v2 string) int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- no need to be public function
- add unit test to verify its behaviour
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2656 +/- ##
==========================================
+ Coverage 38.99% 39.13% +0.13%
==========================================
Files 356 356
Lines 36898 36912 +14
==========================================
+ Hits 14388 14443 +55
+ Misses 20709 20665 -44
- Partials 1801 1804 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
What problem does this PR solve?
ref #2654
What is changed and how it works?
add a function to compare versions by splitting the version into integers to avoid the case like 8.4 > 8.10.
Check List
Tests
Code changes
Side effects
Related changes
Release notes: