ci: drop redundant go build ./... step#93
Merged
Conversation
`go vet ./...` already compiles and type-checks every package (including test files, which `go build` skips), so for the default build it fully subsumes `go build ./...` as a compile check. The only thing build adds is linking the main binaries — and this is a pure-Go tree (modernc.org/ sqlite, no cgo/import "C", no assembly, no //go:linkname), so a successful vet guarantees a successful link. The `go build -tags wailsdesktop` step stays: it compiles tag-gated code vet never sees and links a real cgo/WebKit binary, where link errors are genuinely possible.
There was a problem hiding this comment.
Pull request overview
Removes a redundant compilation check from the CI “build / vet / test” job, relying on existing go vet/go test (and the existing tag-gated desktop build) to keep the same safety guarantees while simplifying the workflow.
Changes:
- Removed the standalone
go build ./...step from the CI test job. - Updated the inline workflow comment to reflect that only the
wailsdesktopbuild needs the installed native dependencies.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
What
Remove the standalone
go build ./...step from thebuild / vet / testCI job.Why
go vet ./...can't analyze code it can't compile, so it already invokes the type-checker on every package — including_test.gofiles, whichgo build ./...skips. For the default build it's a strict superset ofgo build ./...as a compile check. The only thinggo build ./...adds is linking themainbinaries, and that can't fail where compilation succeeds in a pure-Go tree.This repo is pure Go:
modernc.org/sqlite— no cgoimport "C"anywhere*.s) and no//go:linknameSo
go vet ./...passing guaranteescmd/squirrelbuilds. The step was doing nothing the vet step doesn't already do.The
go build -tags wailsdesktop ./cmd/squirrel-desktopstep stays — it's not redundant: it compiles tag-gated codego vet ./...never touches, and links a real cgo/WebKit binary where link errors are genuinely possible. The job name stays accurate since that build remains.This also aligns CI with the local "verify" guidance (vet/test/lint), which intentionally omits
go buildper a standing rule against leaving build artifacts.No coverage lost.