feat(main): forward buildTime ldflag + extract testable helper - #565
Conversation
Addresses two copilot review follow-ups from #564: 1. FormatVersion() previously reported 'built at n/a' even on tagged releases because the template's ldflags injected only main.version + main.build. netresearch/.github#74 extended the template with -X main.buildTime=<commit-timestamp>; this PR adds 'var buildTime string' and forwards it into internal/version.BuildTimestamp so the value lands. 2. The init() forwarding logic is extracted into forwardBuildMetadata(v, b, t), unit-tested in main_test.go against the internal/version package-level vars with snapshot/restore. Empty inputs are treated as 'not injected' and preserve the defaults — locked in by the partial-injection test case. Sync release.yml to the latest template (buildTime ldflag now present). Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
=======================================
Coverage 81.94% 81.94%
=======================================
Files 18 18
Lines 1855 1855
=======================================
Hits 1520 1520
Misses 265 265
Partials 70 70
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the build metadata plumbing so release builds can populate internal/version.BuildTimestamp (via a new main.buildTime ldflag), and refactors the forwarding logic into a unit-testable helper.
Changes:
- Add
buildTimeas a build-injectedmainpackage variable and forward it intointernal/version.BuildTimestamp. - Extract
forwardBuildMetadata(v, b, t)frominit()and add unit tests covering full/empty/partial forwarding. - Update
release.ymlto pass-X main.buildTime=...in ldflags.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/ldap-manager/main.go | Adds buildTime, introduces forwardBuildMetadata, and forwards build metadata during init. |
| cmd/ldap-manager/main_test.go | Adds tests validating forwarding behavior without leaking global state. |
| .github/workflows/release.yml | Passes the new main.buildTime ldflag in the release build matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
) Copilot review on [#565](#565): `internal/version/doc.go` still described ldflags as being applied directly to `internal/version.*`, contradicting the new `main.*` + `forwardBuildMetadata` flow in `cmd/ldap-manager/main.go`. Rewrite the package docstring to describe: - The shared go-app template as the canonical ldflag source (`-X main.version`, `main.build`, `main.buildTime`). - `forwardBuildMetadata()` as the init-time bridge into `internal/version.Version` / `.CommitHash` / `.BuildTimestamp`. - Local-dev default behavior (defaults preserved when shim vars are empty). Also drop the obsolete Makefile / Dockerfile example snippets — the new release pipeline replaces both paths with the shared template, so those sections were actively misleading.
Summary
Addresses copilot-review follow-ups from #564.
1. BuildTimestamp actually populates
FormatVersion()previously reportedbuilt at n/aeven on tagged releases because the template injected onlymain.version+main.build. netresearch/.github#74 extended the template with-X main.buildTime=<commit-timestamp>; this PR addsvar buildTime stringand forwards it intointernal/version.BuildTimestamp.2. Testable forwarding helper
Extracted the three
if X != "" { ... }assignments intoforwardBuildMetadata(v, b, t). Three new test cases inmain_test.go:go run/ untaggedgo buildleaves defaults intactTests use snapshot/restore via
t.Cleanupso they don't leak state.3. Sync release.yml to latest template
Picks up the
-X main.buildTime=ldflag from #74.Test plan
go test -run TestForwardBuildMetadata -v ./cmd/ldap-manager/passes (3 subtests).