-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Check go version #353
Check go version #353
Conversation
@fraenkel Thanks for the PR. The repo has two vendors:
For this PR, since the change is under |
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.
Thank you addressing the comments. Vendor update looks good now. Have couple of minor comments about the code.
} | ||
if !c.Check(ver) { | ||
log.Fatalf("The go version is %v, must be 1.10+", goVersion) | ||
} |
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.
Can we abstract the above code into a separate function ?
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.
ack
cmd/kubebuilder/initproject/init.go
Outdated
log.Fatalf("Could not execute 'go version': %v", err) | ||
} | ||
|
||
goVersion := strings.TrimPrefix(strings.Split(string(out), " ")[2], "go") |
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.
We can get invalid index error here.
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.
ack
Unrelated TestStorageBee failure |
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: fraenkel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
# This is the 1st commit message: Document adding an external type # This is the commit message kubernetes-sigs#2: Fix typo in book: flag is --owner, not --owners # This is the commit message kubernetes-sigs#3: gitbook: fixed installation instructions # This is the commit message kubernetes-sigs#4: Fix domain resource issues in controller - fix in controller-tools and vendor in kubebuilder - bind resource for controller based on whether resource is created. # This is the commit message kubernetes-sigs#5: improve contributing guide # This is the commit message kubernetes-sigs#6: Fix URL for the Gitbook The URL is .io, not .com # This is the commit message kubernetes-sigs#7: Check go version Fixes kubernetes-sigs#353 # This is the commit message kubernetes-sigs#8: Validate create api flags Fixes kubernetes-sigs#321 # This is the commit message kubernetes-sigs#9: Update stable version in book Update newest stable version to "1.0.1". # This is the commit message kubernetes-sigs#10: thirdparty tools: Dockerfile uses k8s 1.11 # This is the commit message kubernetes-sigs#11: build: update cloudbuild config for building tools to use k8s 1.11 # This is the commit message kubernetes-sigs#12: add document for connecting to existing kind # This is the commit message kubernetes-sigs#13: add document for connecting to existing kind # This is the commit message kubernetes-sigs#14: removed company from doc # This is the commit message kubernetes-sigs#15: fixed typo
This does not allow one to use go from tip which has a version of "devel". Wasn't sure if we want to support this.
When I do a dep ensure, a bunch of changes occur in the root Gopkg.toml and vendor directory. Wasn't sure how it was being managed, but I tried to keep it to a minimal change.