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.

Sign up for GitHub

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

Jump to bottom

Added Kustomize, Helm, and Kubectl to argocd version #2329

Merged
merged 12 commits into from Oct 4, 2019
Merged

Added Kustomize, Helm, and Kubectl to argocd version #2329

merged 12 commits into from Oct 4, 2019

Conversation

simster7
Copy link
Member

@simster7 simster7 commented Sep 18, 2019 •
edited

Closes: #2311

argocd-server: v99.99.99+unknown
  BuildDate: 1970-01-01T00:00:00Z
  GitCommit:
  GitTreeState:
  GoVersion: go1.13
  Compiler: gc
  Platform: darwin/amd64
  Ksonnet Version: 0.13.1
  Kustomize Version: 3.1.0
  Helm Version: v2.14.3
  Kubectl Version: v1.15.3

@codecov
Copy link

codecov bot commented Sep 19, 2019 •
edited

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2329   +/-   ##
=======================================
  Coverage   38.53%   38.53%           
=======================================
  Files         103      103           
  Lines       14732    14732           
=======================================
  Hits         5677     5677           
  Misses       8294     8294           
  Partials      761      761

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 b502555...f6acb54. Read the comment docs.

@alexec alexec requested a review from alexmt September 19, 2019 17:03
alexec
alexec reviewed Sep 19, 2019
View reviewed changes
Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

Is there a Golang semver library that you could take advantage of?

util/helm/helm.go Show resolved Hide resolved
util/kustomize/kustomize.go Outdated Show resolved Hide resolved
simster7
simster7 commented Sep 20, 2019
View reviewed changes
util/helm/helm_test.go
func TestVersion(t *testing.T) {
ver, err := Version()
assert.NoError(t, err)
SemverRegexValidation := `^v(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(-(0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(\.(0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*)?(\+[0-9a-zA-Z-]+(\.[0-9a-zA-Z-]+)*)?$`
Copy link
Member Author

Choose a reason for hiding this comment

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

Not very pretty, but the best way I found to validate semvers

jessesuen
jessesuen requested changes Sep 21, 2019
View reviewed changes
server/version/version.go Outdated Show resolved Hide resolved
@alexec
Copy link
Contributor

alexec commented Sep 23, 2019

If you sync with master you may find your build is fixed. Don't forget to re-request a review when you're ready.

@simster7
Copy link
Member Author

simster7 commented Sep 23, 2019 •
edited

Thanks @alexec. Looks like we're good for the re-review, @jessesuen

@alexec
Copy link
Contributor

alexec commented Oct 2, 2019

@jessesuen been off for a few days.

jessesuen
jessesuen approved these changes Oct 2, 2019
View reviewed changes
Copy link
Member

@jessesuen jessesuen left a comment

Choose a reason for hiding this comment

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

LGTM. just resolve conflicts and we'll merge it in.

@simster7
Copy link
Member Author

simster7 commented Oct 3, 2019

@alexec @jessesuen This should be good to go!

@alexec
Copy link
Contributor

alexec commented Oct 3, 2019

@jessesuen do you want to merge?

@jessesuen jessesuen merged commit f185137 into argoproj:master Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jessesuen jessesuen jessesuen approved these changes

@alexmt alexmt Awaiting requested review from alexmt

@alexec alexec Awaiting requested review from alexec

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Surface tooling versions in /api/version
3 participants
@simster7 @alexec @jessesuen