-
Notifications
You must be signed in to change notification settings - Fork 78
chore: Bump k8s dependencies to 1.23 #212
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
chore: Bump k8s dependencies to 1.23 #212
Conversation
3831f75
to
43406fb
Compare
Unrelated to this PR, but is there anything blocking us from removing the |
+1 to removing the vendor directory from this repository. I don't think it makes a ton of sense upstream, and would simplify the downstreaming process (e.g. we run into less merge conflicts). Do we typically bump this repositories' minor version tag when we cut new k8s dependencies? If so, it might be worth trying to see whether purging the vendor directory is a small amount of work and piggy back off that new minor version. |
Some notable bumps in versions: 1. Controller-runtime to 0.11.0 2. Controller-tools to 0.8.0 3. K8s modules to 1.23+ 4. Go to 1.17
43406fb
to
4bd7a1e
Compare
I have no problems with removing vendor as long as things continue to work the way they are (in CI and etc). |
Took a stab at removing the vendor directory in #213. It would real nice to also remove it from the registry repository as well (we're tracking the OLM removal as an upstream issue, but it's a bit more nuanced to remove it than I'd like). |
@@ -14,7 +14,7 @@ jobs: | |||
- name: Set up Go | |||
uses: actions/setup-go@v2 | |||
with: | |||
go-version: 1.16 |
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.
super nit (feel free to ignore): It would be nice to separate these types of changes into their own commit so viewing the overall diff outside of vendor changes doesn't result in a decent lag in the github UI.
/lgtm |
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.
/approved
/lgtm |
/approve |
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.
Approved.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, dinhxuanvu, varshaprasad96 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 |
Some notable bumps in versions: