chore: bring the gate up to the standard of the sibling packages - #30
Merged
Conversation
CI ran tests, a race pass and golangci-lint, and nothing else. It reported no
vulnerable dependencies, no licence problems and no formatting drift because it
never looked for any of them.
Added, ported from cboxdk/fpm-exporter:
- govulncheck
- a deterministic CycloneDX 1.5 SBOM (tools/sbomnorm normalises the
generator's host-dependent fields, so the committed file only changes when
dependencies do)
- a dependency licence check (tools/licensecheck) reading that SBOM rather
than the network, so CI and offline agree. 63 dependencies, all permissive,
no exceptions needed.
- gofmt and `go mod tidy` drift checks
- `make check`, running the identical set locally
Enabled bodyclose, errorlint and misspell -- one at a time after fixing what
each surfaced, which is the policy this repo's own .golangci.yml states. Seven
findings: three real fixes (a type assertion and two == comparisons on errors,
all of which stop matching once an error is wrapped), and four documented as
deliberate. Of those, three are in the snapshot client, where wrapping the cause
with %w instead of %v would make it matchable and defeat the stated design that
callers test only for ErrUnavailable; the fourth is a bodyclose false positive
where the response body is the stream and is closed by the reader goroutine.
26 files were not gofmt-clean.
The PHPeek name survived the rebrand in six places, two of them user-visible:
the TUI header and its keyboard-shortcut screen, the audit log's start and
shutdown records, the build-info metric's help text, and a config warning.
Scale-to-zero was treated as untouchable. Across internal/snapshot,
internal/process and internal/autotune the only production change is one line in
reap.go -- `err == syscall.ECHILD` becomes `errors.Is(err, syscall.ECHILD)`,
equivalent for the bare errno Wait4 returns and correct if it is ever wrapped.
Everything else there is comments and test formatting. Verified with
`go test -race` over both packages in full.
…pted Two failures from the previous commit, both mine. The licence checker's exception `reason` field was never read here: this repo needs no exceptions, so the map is empty and the field was dead. It now prints each accepted exception with its reason, which is what an operator actually wants to see -- a count says nothing about whether the reasoning still holds. govulncheck failed on six standard-library advisories. `go-version: '1.26'` resolved to 1.26.5, and all six are fixed in 1.26.6 -- so the scan job was running on a toolchain that still shipped them. Pinned to the patch. My earlier claim that CI was unaffected because it tracked the latest patch was wrong: released binaries were built on 1.26.5 too.
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.
CI ran tests, a race pass and golangci-lint, and nothing else. It reported no
vulnerable dependencies, no licence problems and no formatting drift — because
it never looked for any of them.
The gate
Ported from
cboxdk/fpm-exporter, where these were built last week:cyclonedx-gomodrecords thepseudo-version of HEAD, tags purls with the generating host's
goos/goarchand embeds hashes of its own binary — so a laptop and a runner could never
agree.
tools/sbomnormnormalises all three; the committed file changes onlywhen dependencies do.
CI and offline give the same answer. 63 dependencies, all permissive, no
exceptions needed (fpm-exporter needs one, for gopsutil's MPL-2.0
Apple-Silicon helper; nothing here does).
go mod tidydrift checks. 26 files were not gofmt-clean.make check, running the identical set locally.Linters
bodyclose,errorlintandmisspell— added one at a time after fixing whateach surfaced, which is what this repo's own
.golangci.ymlsays to do. Sevenfindings, three real:
handleExecutionErrortype-asserted on the error instead oferrors.As==/!=comparisons against sentinel errorsFour are documented as deliberate rather than changed. Three are in the snapshot
client, where the existing comment explains that the cause is joined with
%von purpose: wrapping it with
%wwould make it matchable and defeat the designthat callers test for
ErrUnavailableand nothing else. The fourth is abodyclosefalse positive where the response body is the stream and is closedby the reader goroutine.
Branding
The PHPeek name survived the rebrand in six places, two of them user-visible:
the TUI header, its keyboard-shortcut screen, the audit log's start and shutdown
records, the build-info metric's help text, and a config warning.
Scale-to-zero was treated as untouchable
Across
internal/snapshot,internal/processandinternal/autotunethere isexactly one production line changed:
Equivalent for the bare errno
Wait4returns, and correct if it is everwrapped. Everything else in those packages is comments and test formatting.
go test -racepasses over bothinternal/snapshotandinternal/processinfull, not just
-short.Note on govulncheck's first run
It reports seven standard-library advisories against a local Go 1.26.4. All
are fixed in 1.26.6, and CI pins
'1.26', which resolves to the latest patch —so released binaries are unaffected. Worth knowing that the answer was simply
never being asked before.