chore: bump minimum Go version to 1.25 - #472
Merged
Merged
Conversation
The `go` directive was pinned at 1.23.12 while every current consumer of this library is already on 1.25 or newer (terraform-provider 1.25.8, cli 1.26.6, argocd-gateway 1.26.0). Move the floor to 1.25.0 — new enough to clear the stale pin, conservative enough that no consumer has to bump. From language version 1.24 onward, vet's printf analyser reports non-constant format strings, which flagged 43 `jsonassert.Assertf` call sites across the test suite and would have failed the CI vet step. Each now passes the serialised JSON as a `%s` argument rather than as the format string itself. `Assertf` calls `fmt.Sprintf(expectedJSON, args...)` unconditionally, so this is behaviour-preserving for `%`-free input and strictly safer otherwise. No production code was affected. Also pin the CI toolchain to 1.27.x instead of `>=1.19.0`, so a new Go release is a deliberate change rather than a silent one, and bump setup-go v3 -> v5. Verified: `go build -race ./...` and `go vet ./...` clean under both go1.26.3 and go1.27.0; all 43 affected marshalling tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TestPackageServiceUploadDelta_UploadedDelta hardcoded the byte sizes of a zip built from random content. archive/zip's per-entry overhead changed in Go 1.27 (582 -> 346 bytes for this two-entry archive), so pinning CI to 1.27.x made the assertion fail with 1048922 != 1049158. The comment above these lines already said the values were machine-specific and "left for explanatory purposes only" — the sibling test TestPackageServiceUploadDelta_NotEfficient has the equivalent assertions commented out. This one kept them live, which looks like an oversight. Assert what the test is actually about instead: only content2.txt is new between the two uploads, so the delta should carry roughly half the full file rather than all of it. Both observed toolchain outputs satisfy the new bounds comfortably. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NickJosevski
enabled auto-merge (rebase)
August 27, 2026 07:11
YuKitsune
approved these changes
Aug 27, 2026
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.
Why
The
godirective has been pinned at1.23.12while every current consumer is already well ahead of it:clioctopus-argocd-gatewayterraform-provider-octopusdeployBecause this is a public library, the
godirective is the minimum version imposed on every consumer, so this stops short of the latest (1.27) deliberately —1.25.0clears the stale pin without forcing the Terraform provider to bump before it can take a new client release.No dependency required this: the highest minimum across the module graph is
go 1.23.0(golang.org/x/text,net,crypto,sys).The vet fallout
From language version 1.24 onward, vet's
printfanalyser reports non-constant format strings. That flags 43jsonassert.Assertfcall sites, which would fail thego vet -v ./...CI step:Each is fixed the same way:
This is behaviour-preserving.
Assertfdoesfmt.Sprintf(expectedJSON, fmtArgs...)unconditionally, so with no args and no%in the string the result is identical — and strictly safer if serialised JSON ever contains a%. All 43 are in_test.gofiles; no production code is affected.CI
Pinned to an explicit
1.27.xinstead of>=1.19.0, which silently tracked whatever was newest — that's how a toolchain bump surprises you rather than being a deliberate change. Also bumpsactions/setup-gov3 → v5.Verification
go build -race ./...— cleango vet ./...— clean under both go1.26.3 and go1.27.0 (the two toolchains flag slightly different subsets; the fix covers the union)gofmt -l— cleango mod tidyproduces nogo.sumchurnNote: unit and integration tests aren't separated in this repo, so a full local
go test ./...can't pass without a live Octopus instance. The integration suite needs a CI run to confirm.🤖 Generated with Claude Code