Repository navigation
build: restore Go 1.20 minimum - #57
Merged
Merged
Conversation
There was a problem hiding this comment.
Review: build: restore Go 1.20 minimum
Clean, well-scoped change. Lowering the go.mod floor to go 1.20 so downstream modules don't have to raise their minimum, and validating both the new floor and a modern leg in CI, is the right approach. fail-fast: false, the per-leg name: build (Go ${{ matrix.go }}), and the setup-go v4->v5 bump (aligning the build job with the existing llgo-test job) are all good.
Verified locally
GOTOOLCHAIN=local go build ./...passes under the Go 1.20 language version — the downgrade compiles.- No Go 1.21+ language/stdlib features in the source (no
min/max/clearbuiltins, noslices/maps/cmpimports), so 1.20 is a safe floor. - No README/docs/badge in the repo asserts a minimum Go version, so nothing is left stale by the directive change.
Note (non-blocking)
- The new
go test ./...step is effectively a compile smoke check onubuntu-latest: 13 of 15_test.gofiles are gated behind//go:build llgo/windows, so onlycandc/bitfieldactually run tests on the standard runner, and the step largely recompiles the same packages as the precedinggo build -v ./.... That's fine for a build-health gate — just flagging that it doesn't broaden real test execution.
Minor, optional suggestions are inline. None block merging.
This file contains hidden or 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
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.
The Go 1.23 requirement was introduced with the qiniu/x dependency in #36 and remained after #41 removed that dependency. Restore
go 1.20so downstream modules can use the current native bindings without raising their minimum Go version.Build and test with Go 1.20 and the latest stable Go in CI, with automatic toolchain switching disabled. Go 1.20 is retained for downstream compatibility verification; the stable job checks the current supported toolchain. These jobs validate the library and do not publish release artifacts. Update that job's setup-go action to v5, matching the existing LLGo job. No library source changes are needed. The ordinary Go suite executes the tests in c and c/bitfield and compiles the remaining packages; the separate LLGo jobs exercise the native bindings.
Validation on macOS arm64, based on main
1e54359:go build ./...(49 packages),go test ./..., andgo mod tidy -go=1.20pass.go test ./...passes.git diff --checkandactionlint -ignore 'SC2155:'pass. The ignore covers only the three existing SC2155 warnings in the unchanged LLGo build script.Additional Linux/Windows amd64 cross-builds with CGO disabled encounter existing missing declarations (
SigjmpBufandnet.EAI_*). The originalgo 1.23module with Go 1.23.12 produces identical errors; these are not regressions from the version change. The previous CI run passed both ordinary Go jobs, native LLGo tests on Ubuntu/macOS, and the Windows LLVM demangle bridge; CI will revalidate the updated matrix.