-
Notifications
You must be signed in to change notification settings - Fork 473
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
Build: Upgrade to Golang 1.23 #6169
base: master
Are you sure you want to change the base?
Conversation
@@ -69,14 +69,14 @@ func findRootKeys(algodDir string) []*crypto.SignatureSecrets { | |||
var handle db.Accessor | |||
handle, err := db.MakeErasableAccessor(path) | |||
if err != nil { | |||
return nil //nolint:nilerr // don't care, move on | |||
return nil |
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.
Why is the nilerr linter willing to allow this now?
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.
cmd/loadgenerator/main.go:72:15: directive `//nolint:nilerr // don't care, move on` is unused for linter "nilerr" (nolintlint)
return nil //nolint:nilerr // don't care, move on
^
cmd/loadgenerator/main.go:79:15: directive `//nolint:nilerr // don't care, move on` is unused for linter "nilerr" (nolintlint)
return nil //nolint:nilerr // don't care, move on
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 get it. That's nilerr
's whole job.
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 started warning me for having it, warning went away when I removed. Given what this code does, I wasn't too concerned.
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.
Yes, the code is certainly correct. But I don't see how nilerr
knows that. It's doing exactly the thing nilerr
thinks is bad, because it usually is.
@cce knows linters well. Any ideas?
one new thing in go 1.23 is the golang.org/x/exp/maps and golang.org/x/exp/slices packages are provided as builtin "maps" and "slices" packages now, so we don't need the golang.org/x/exp dependency. |
If I remember correctly, there was at least one function that changed
between the `exp` version and being included officially. Does that ring a
bell? I can't recall what it was.
…On Thu, Nov 14, 2024, 2:42 PM cce ***@***.***> wrote:
one new thing in go 1.23 is the golang.org/x/exp/maps and
golang.org/x/exp/slices packages are provided as builtin "maps" and
"slices" packages now, so we don't need the golang.org/x/exp dependency.
—
Reply to this email directly, view it on GitHub
<#6169 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADL7TZUNRLAR55LOIZSGZL2AT4LRAVCNFSM6AAAAABRX5PLI6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINZXGI3DONRSGM>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
… block-generator.
@@ -423,7 +423,6 @@ byte 0x%s | |||
{pkTampered2, false}, | |||
} | |||
for i, test := range decompressTests { | |||
i, test, source := i, test, source |
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.
src := fmt.Sprintf(source
below now modifies the original in parallel t.Run() captures the source
programsource
content after modification below the loop
Summary
This upgrades go-algorand to Golang 1.23.3, including necessary linter and Dockerfile updates.
Should close #6151 as well.
Test Plan
Existing tests/test suite all should pass.