-
Notifications
You must be signed in to change notification settings - Fork 20
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 up to golang 1.23 version #206
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Sam Yuan <[email protected]>
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'm not convinced it's worth moving the minimum supported Go version up unless:
- it is necessary to meet Go version requirements of dependencies; or
- we are going to release a new version and want to minimise the different versions of Go we support and test with
|
||
require ( | ||
github.com/IBM/sarama v1.43.1 | ||
github.com/Knetic/govaluate v3.0.0+incompatible | ||
github.com/golang/mock v1.1.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.
We should be using go.uber.org/mock
instead of the (deprecated) github.com/golang/mock
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.
graph.txt
I don't know why but... it seems that we have github.com/golang/mock
as a part of go mod, after go mod tidy
so I am going to use
go mod edit -replace=github.com/golang/[email protected]=github.com/uber-go/[email protected]
to apply go.uber.org/mock
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 don't see that behaviour. I wonder if you have locally some generated mocks that were created using github.com/golang/mock? If you get rid of the generated mocks, either with find . -name '*_mock_test.go' -delete -print
or git clean -xdf
, then run make generate
again, you should get mocks only using go.uber.org/mock.
From this clean state, running either go get ./...
or go mod tidy
should produce no changes to the go.mod and go.sum files.
@@ -1,10 +1,11 @@ | |||
module github.com/hyperledger/fabric-admin-sdk | |||
|
|||
go 1.21 | |||
go 1.23 |
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 not move the minimum required Go version beyond 1.22 while 1.22 is still a supported Go version
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 suspect that having a 2-digit go
directive and no toolchain
directive will cause problems for consumers. I think we don't see that in our tests since the e2e tests are within the same module so do not exercise the API in the same way that an end user would.
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.
ok, I will update to go 1.22 later.
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 go.mod is only really supposed to be updated using the go CLI commands now. To update the Go version to 1.22 with no automatically added toolchain directive, you probably need go get [email protected] toolchain@none
. See the Go Toolchains documentation; particularly the Managing Go version module requirements with go get section.
ref to https://go.dev/doc/devel/release#policy
bump to golang 1.23 from golang 1.21