-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🌱 Bump github.com/blang/semver to v4 #9189
🌱 Bump github.com/blang/semver to v4 #9189
Conversation
176954d
to
a314957
Compare
/retest |
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
LGTM label has been added. Git tree hash: 7111bcb207d2699937684251f379a3758024d816
|
@@ -56,6 +55,7 @@ require ( | |||
github.com/antlr/antlr4/runtime/Go/antlr v1.4.10 // indirect | |||
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a // indirect | |||
github.com/beorn7/perks v1.0.1 // indirect | |||
github.com/blang/semver/v4 v4.0.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.
Can you open a follow-up PR to regenerate all go.mod files? (i.e. delete everything except the first 3 lines in go.mod + delete go.sum and run the generate target)
I think our current blocks are a bit arbitrary and there are plenty of interesting diffs when we regenerate
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.
One major diff is that we get a lot less hashicorp dependencies
(cc @killianmuldoon)
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.
Interesting data point. Top-level go.sum goes from 1011 to 856 lines
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.
Probably also something that we want to repeat in CAPV
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.
This is already done in capv via kubernetes-sigs/cluster-api-provider-vsphere#2219
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.
When re-generating go.mod it seems to change to a slightly different module
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.
Or maybe I just did something wrong, let's see :D
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.
From trying this: my conclusion is: not worth it.
The bumps we get are:
google.golang.org/grpc v1.55.0 => v1.57.0
k8s.io/api v0.27.2 => v0.27.4
k8s.io/apiextensions-apiserver v0.27.2 => v0.27.4
k8s.io/apimachinery v0.27.2 => v0.27.4
k8s.io/apiserver v0.27.2 => v0.27.4
k8s.io/client-go v0.27.2 => v0.27.4
k8s.io/cluster-bootstrap v0.27.2 => v0.27.4
k8s.io/component-base v0.27.2 => v0.27.4
k8s.io/klog/v2 v2.90.1 => v2.100.1
k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f => v0.0.0-20230811205723-7ac0aad8c58d
k8s.io/kubectl v0.27.2 => v0.27.4
k8s.io/utils v0.0.0-20230209194617-a36077c30491 => v0.0.0-20230726121419-3b25d923346b
Lot's of other dependencies would get downgraded.
Including github.com/coredns/corefile-migration
which reduces some transitive deps to hashicorp (but not all). And I assume we don't want to downgrade that.
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.
Definitely don't want to downgrade. I was just looking at our current go.mod file and the 3 different require sections seems to be pretty arbitrary. Since this PR the 2nd one even has a combination of direct & indirect dependencies.
Maybe we can at least put all our current dependencies in the same block and let go mod tidy clean it up?
As far as I can tell this should cleanly format into two blocks. One for direct one for indirect dependencies
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.
(basically the equivalent of enforcing go import formatting)
/approve Thank you very much! Nice cleanup |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
What this PR does / why we need it:
Bumps
github.com/blang/semver
everywhere to use v4.https://github.com/blang/semver/releases/tag/v4.0.0
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
/area dependency