-
Notifications
You must be signed in to change notification settings - Fork 184
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
Adopt go 1.21 #2614
Adopt go 1.21 #2614
Conversation
This is really cool! |
Happy to hold off on this until |
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.
One nit on environment variable hygiene as I use ci-build.ps1
to build locally (not sure if anyone else does). Otherwise looks good.
Victor should give the final approval as getting the linting tools in order are the critical path here.
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).
2cb90f5
to
8cb63e1
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSIContainer
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Updated to the latest golangci-lint, which now supports go 1.21. |
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.
ci-build.ps1 and other eng changes LGTM.
This change moves us to use Go 1.21 for building
azd
.As part of go 1.21, the
maps
,slices
andslog
packages, previously ingolang.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 standardlibrary. 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 themaps
package 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.mod
to declare that we need at least go 1.21 to buildazd
. Developers which haven't upgraded will see build errors due to references ofmaps
,slices
andlog/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).