Conversation
The Homebrew formula switched to source builds, but the renderer only passed the semantic version into the Go ldflags. As a result, brew installs reported fallback commit/date values even though release artifacts already had full metadata. This updates the release workflow to resolve the tagged commit and commit timestamp, passes both values into the formula renderer, and adds a regression check for the generated formula so the metadata wiring stays covered in CI. Constraint: Homebrew installs now build from the tagged source tarball instead of shipping unsigned macOS binaries Rejected: Change cmd/version.go defaults | would hide packaging regressions instead of fixing the formula path Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep Homebrew formula ldflags aligned with GoReleaser version metadata whenever release packaging changes Tested: bash test/render_homebrew_formula.sh Tested: go test ./... Tested: go vet ./... Tested: go build ./... Tested: goreleaser check Not-tested: golangci-lint run (fails in this repo with existing "no go files to analyze" context loading error)
There was a problem hiding this comment.
Code Review
This pull request updates the Homebrew formula generation script to include commit hash and build date metadata in the git-volume binary via ldflags. It also introduces a new regression test script to verify the generated formula's content. The review feedback suggests replacing assert_match with assert_includes in the generated formula's test block to prevent potential regex matching errors caused by special characters in date strings, along with a corresponding update to the regression test's grep patterns.
The generated Homebrew formula test compared version metadata with assert_match, which treats string inputs as regular expression patterns. ISO8601 build timestamps commonly include '+' timezone offsets, so the check could fail even when the output was correct. Switch the generated test to assert_includes and update the renderer regression script to lock in the literal substring checks suggested in review. Constraint: Homebrew formula tests must safely handle ISO8601 build timestamps emitted by the release workflow Rejected: Escape the generated strings for regex use | more fragile than literal substring assertions for this case Confidence: high Scope-risk: narrow Reversibility: clean Directive: Prefer literal output assertions over regex matching for generated release metadata unless pattern matching is required Tested: bash test/render_homebrew_formula.sh Tested: bash -n scripts/render-homebrew-formula.sh test/render_homebrew_formula.sh Tested: go test ./... Not-tested: go vet ./... (unchanged from prior passing state for this review-only follow-up) Not-tested: goreleaser check (unchanged from prior passing state for this review-only follow-up)
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the Homebrew formula rendering script to include commit hash and build date metadata via Go linker flags. It also adds a new test script to verify that the formula is correctly generated with these new fields. A review comment suggests quoting the values within the ldflags to ensure that metadata containing spaces, such as dates, is correctly handled by the Go linker.
Summary
Testing
Closes #35