-
Notifications
You must be signed in to change notification settings - Fork 256
🌱 Upgrade bingo tooling deps #1703
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
🌱 Upgrade bingo tooling deps #1703
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1703 +/- ##
=======================================
Coverage 55.17% 55.17%
=======================================
Files 136 136
Lines 15918 15918
=======================================
Hits 8783 8783
Misses 5982 5982
Partials 1153 1153 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8ca6d09
to
9f65c83
Compare
.bingo/variables.env
Outdated
|
||
GORELEASER="${GOBIN}/goreleaser-v1.26.2" | ||
|
||
PROTOC_GEN_GO_GRPC="${GOBIN}/protoc-gen-go-grpc-v1.3.0" | ||
PROTOC_GEN_GO_GRPC="${GOBIN}/protoc-gen-go-grpc-v1.5.1" |
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.
@joelanford @grokspawn the methods changed for the latest, but I think we need to move forward right.
Could you please give a look on this one?
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.
Generally speaking, we don't ever want to change the protoc-gen, since our GRPC API is public and part of OLMv0 which is in maintenance mode.
That's why we pinned this one.
The other bumps look OK, but I'll
/hold
this until we get to consensus about this one.
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.
I think it is a good thought.
I was concerned about it.
I will revert it so we can move forward 👍
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 was reverted so I think we are safe here to
/hold cancel
GINKGO := $(GOBIN)/ginkgo-v2.23.4 | ||
$(GINKGO): $(BINGO_DIR)/ginkgo.mod | ||
@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies. | ||
@echo "(re)installing $(GOBIN)/ginkgo-v2.22.2" | ||
@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=ginkgo.mod -o=$(GOBIN)/ginkgo-v2.22.2 "github.com/onsi/ginkgo/v2/ginkgo" | ||
@echo "(re)installing $(GOBIN)/ginkgo-v2.23.4" | ||
@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=ginkgo.mod -o=$(GOBIN)/ginkgo-v2.23.4 "github.com/onsi/ginkgo/v2/ginkgo" |
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 should also bump ginkgo
in our go.mod to match.
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 is updated in the go.mod already
github.com/onsi/ginkgo/v2 v2.23.4
|
||
require github.com/onsi/ginkgo/v2 v2.22.2 // ginkgo | ||
toolchain go1.24.3 |
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 remove the toolchain line? Or does the dependency force it to be pulled in? (same question for all the dep bump mod files).
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 fails with
to latest...
Error: get: golangci-lint.mod: getting github.com/golangci/golangci-lint/cmd/golangci-lint@latest: install: list: go: golangci-lint.tmp.mod requires go >= 1.24.3 (running go 1.24.2)
: exit 1
Usage:
bingo get [flags] [<package or binary>[@version1 or none,version2,version3...]]
I think we need to ensure we go to 1.24.3 because of the downstream, right?
In downstream we have 1.24.3
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.
The value of the toolchain in the bingo files is the value for the operator-registries go version, which is 1.24.3. If this was updated to 1.24.4, the toolchain referenced here would be 1.24.4.
Hi @grokspawn Now, we can merge this one 👍 |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: acornett21, perdasilva 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 |
1a0ba3f
into
operator-framework:master
No description provided.