-
Notifications
You must be signed in to change notification settings - Fork 12
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
updating k8s to 1.30 and go to 1.22.5 #158
updating k8s to 1.30 and go to 1.22.5 #158
Conversation
Signed-off-by: Adam D. Cornett <[email protected]>
@@ -58,7 +58,7 @@ endif | |||
# Image URL to use all building/pushing image targets | |||
IMG ?= $(IMAGE_TAG_BASE):$(RELEASE_TAG) | |||
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. | |||
ENVTEST_K8S_VERSION = 1.29.0 | |||
ENVTEST_K8S_VERSION = 1.30.0 |
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.
For all of these... I'd like to see these turned into ?= assignments so that we can define them outside of the Makefile, to be able to test the versions in CI without having to actually create a PR to do it. Or even have a current/next type of deal so we can be testing both on a rolling basis. Just a though. Not necessarily relevant to this PR. 😄
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.
It looks like this might be the only one that does not have a ?= assignment.
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.
That does look like the only one. I like the thought of what you are saying, the only caveat to doing this or that testing, is that most of these dependencies need to be in lock-step with one another to work properly. Sometimes there is variances, but usually not from what I've seen.
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.
/lgtm
deadcode
changed tounused