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

cluster, dm: format semver input #1009

Merged
merged 11 commits into from
Dec 29, 2020
Merged

Conversation

AstroProfundis
Copy link
Contributor

What problem does this PR solve?

  1. prepend v to bare version number, so that v4.0.8 and 4.0.8 are both valid input
  2. validate input version to ensure it's a valid semver string

Close #350

Check List

Tests

  • Integration test
  • Manual test (add detailed scripts or steps below)

Release notes:

cluster, dm: support version input without leading 'v'

@AstroProfundis AstroProfundis added type/enhancement Categorizes issue or PR as related to an enhancement. category/usability Categorizes issue or PR as a usability enhancement. labels Dec 24, 2020
@AstroProfundis AstroProfundis self-assigned this Dec 24, 2020
@ti-chi-bot ti-chi-bot requested review from lonng and lucklove December 24, 2020 08:24
@ti-chi-bot ti-chi-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 24, 2020
@codecov-io
Copy link

codecov-io commented Dec 24, 2020

Codecov Report

Merging #1009 (4645a43) into master (b693df4) will increase coverage by 0.06%.
The diff coverage is 45.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1009      +/-   ##
==========================================
+ Coverage   55.63%   55.69%   +0.06%     
==========================================
  Files         280      280              
  Lines       19741    19749       +8     
==========================================
+ Hits        10982    10999      +17     
+ Misses       7049     7043       -6     
+ Partials     1710     1707       -3     
Flag Coverage Δ
cluster 43.64% <25.00%> (+0.11%) ⬆️
dm 23.98% <60.00%> (+0.01%) ⬆️
integrate 49.97% <45.45%> (+0.09%) ⬆️
playground 20.31% <0.00%> (-0.01%) ⬇️
tiup 16.48% <0.00%> (-0.01%) ⬇️
unittest 22.27% <18.18%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/cluster/command/deploy.go 64.28% <33.33%> (-3.22%) ⬇️
components/cluster/command/upgrade.go 75.00% <33.33%> (-10.72%) ⬇️
components/dm/command/deploy.go 68.75% <33.33%> (-4.59%) ⬇️
pkg/utils/semver.go 100.00% <100.00%> (ø)
pkg/cluster/spec/tiflash.go 73.69% <0.00%> (+0.86%) ⬆️
pkg/cluster/api/pdapi.go 59.93% <0.00%> (+1.26%) ⬆️
pkg/cluster/spec/pd.go 69.87% <0.00%> (+2.56%) ⬆️
pkg/cluster/template/scripts/pd.go 71.87% <0.00%> (+3.12%) ⬆️
pkg/utils/http_client.go 72.97% <0.00%> (+5.40%) ⬆️

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 b693df4...4645a43. Read the comment docs.

@lucklove
Copy link
Member

/lgtm

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 24, 2020
@lucklove
Copy link
Member

/merge

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 24, 2020
@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: afe2b12

@ti-chi-bot
Copy link
Member

@AstroProfundis: Your PR has out-of-dated and I have automatically updated it for you.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/ti-community-prow repository.

@ti-chi-bot ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed status/can-merge Indicates a PR has been approved by a committer. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 28, 2020
@lucklove
Copy link
Member

/merge

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 29, 2020
@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 4645a43

@ti-chi-bot ti-chi-bot merged commit 3701f8a into pingcap:master Dec 29, 2020
@AstroProfundis AstroProfundis deleted the fmtver branch December 29, 2020 08:42
@lucklove lucklove added this to the v1.3.1 milestone Dec 31, 2020
lucklove added a commit that referenced this pull request Dec 31, 2020
* cluster, dm: format semver input

* utils: filter nightly version

* cluster: fix ci test

Co-authored-by: SIGSEGV <[email protected]>
Co-authored-by: Ti Prow Robot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/usability Categorizes issue or PR as a usability enhancement. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/enhancement Categorizes issue or PR as related to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check version valid for deploy command
4 participants