Skip to content
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

multi: Use go 1.16 features. #2722

Merged
merged 5 commits into from Sep 1, 2021
Merged

multi: Use go 1.16 features. #2722

merged 5 commits into from Sep 1, 2021

Conversation

jholdstock
Copy link
Member

@jholdstock jholdstock commented Aug 27, 2021

Now that Go 1.16 is the minimum supported version, we can take advantage of new features introduced in 1.16.

  • GO111MODULE environment variable now defaults to "on"
  • "go build" and "go test" now exit with an error rather than silently modifying go.mod or go.sum files
  • Don't use deprecated ioutil package

Also updating README and some Dockerfiles which were still referencing older versions of Go.

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit torn on parts of this to be honest. We officially require newer versions of Go, but typically I prefer the code to unofficially work with older versions until there is some feature that is specifically required which means we really do need a newer version.

That said, I did notice it is changing the code without update the various go.mod files to require the new minimum version those changes introduce. Updating a go.mod to require a higher minimum version is, to me anyway, a red flag that should be carefully considered because it hoists all dependents into using a new Go version too.

EDIT: To be clear, the only commit I have any issue with is multi: Don't use deprecated ioutil package. The rest of them look fine (minus the ordering issue I called out elsewhere).

Dockerfile Show resolved Hide resolved
dcrec/secp256k1/loadprecomputed.go Show resolved Hide resolved
rpcclient/infrastructure.go Show resolved Hide resolved
@jholdstock
Copy link
Member Author

Reordered commits. Updated go.mod (which also caused some go.sum changes).

Ready for another look.

@davecgh davecgh changed the title Use go 1.16 features multi: Use go 1.16 features. Aug 31, 2021
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good code wise now. The commit messages need to be updated to follow the model git commit messages format in the code contribution guidelines though. Specifically, it looks like they aren't wrapped.

@davecgh davecgh added this to the 1.7.0 milestone Aug 31, 2021
Update Dockerfiles to use golang:1.17 as their base/build image, rather
then golang:1.15.
As of go 1.16, the GO111MODULE environment variable defaults to "on",
so it doesn't need to be set explicitly when building or testing.
As of go 1.16, the "io/ioutil" package is deprecated. All functionality
from "io/ioutil" has been moved to either the "io" or "os" packages.
As of go 1.16, "go build" and "go test" now exit with an error rather
than silently modifying go.mod or go.sum files. As a result, we don't
need to manually check for altered mod/sum files in the test script.
Since the release of go 1.17, the supported versions listed in the
README should be 1.16 and 1.17.
@davecgh davecgh merged commit 960f9e8 into decred:master Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants