Adopt go 1.21 #2614
Merged
Adopt go 1.21 #2614
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ellismg
requested review from
danieljurek,
wbreza,
vhvb1989,
hemarina and
weikanglim
as code owners
|
This is really cool! |
|
Happy to hold off on this until |
danieljurek
reviewed
Aug 9, 2023
danieljurek
reviewed
Aug 9, 2023
This change moves us to use Go 1.21 for building `azd`. As part of go 1.21, the `maps`, `slices` and `slog` packages, previously in `golang.org/x/exp` have been moved into the standard libary. As part of this change, I've moved to use these versions of the packages instead of using the experimental versions. However, there were a few places where we were using `maps.Keys`, which was not added to the standard library. https://go-review.googlesource.com/c/go/+/513715 explains the rationale here, they may want `Keys` to return an iterator, and were not comfortable locking the name at this time. This functionality will be added to the `maps` package in a future release, it seems, likely under a name like `KeysSlice`. For now, I just kept the existing use of the the experimental package. We can update the import path and move to the stdlib version when it lands later. This change also updates `go.mod` to declare that we need at least go 1.21 to build `azd`. Developers which haven't upgraded will see build errors due to references of `maps`, `slices` and `log/slog` which do not exist in earlier versions of the standard libary. I've also enabled the loopvar experiment for our CI builds and tests. https://github.com/golang/go/wiki/LoopvarExperiment gives more information on what this does, but the long and short of it is that this behavior is likely to become the default in a future version of go, and the semantics of it more closely match what developers expect, so I would like us to just lock in the changes now (we are presently clean on this, so there are no changes, I just don't want us to introduce any new bad uses).
ellismg
force-pushed
the
ellismg/adopt-go-1-21
branch
from
2cb90f5
to
8cb63e1
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSIContainerDocumentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
|
Updated to the latest golangci-lint, which now supports go 1.21. |
weikanglim
approved these changes
Aug 14, 2023
danieljurek
approved these changes
Aug 15, 2023
vhvb1989
approved these changes
Aug 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change moves us to use Go 1.21 for building
azd.As part of go 1.21, the
maps,slicesandslogpackages, previously ingolang.org/x/exphave been moved into the standard libary. As part of this change, I've moved to use these versions of the packages instead of using the experimental versions.However, there were a few places where we were using
maps.Keys, which was not added to the standardlibrary. https://go-review.googlesource.com/c/go/+/513715 explains the rationale here, they may want
Keysto return an iterator, and were not comfortable locking the name at this time. This functionality will be added to themapspackage in a future release, it seems, likely under a name likeKeysSlice. For now, I just kept the existing use of the the experimental package. We can update the import path and move to the stdlib version when it lands later.This change also updates
go.modto declare that we need at least go 1.21 to buildazd. Developers which haven't upgraded will see build errors due to references ofmaps,slicesandlog/slogwhich do not exist in earlier versions of the standard libary.I've also enabled the loopvar experiment for our CI builds and tests. https://github.com/golang/go/wiki/LoopvarExperiment gives more information on what this does, but the long and short of it is that this behavior is likely to become the default in a future version of go, and the semantics of it more closely match what developers expect, so I would like us to just lock in the changes now (we are presently clean on this, so there are no changes, I just don't want us to introduce any new bad uses).