Warn on build toolchain version drift (#27) #77
No reviewers
Labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
keyop-go/nfq_forwarder!77
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "issue27_toolchain_versioning"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #27.
Adds
.tool-versionspinning exactgo/golangci-lint/goreleaser/gitleaks/yamllintversions, andscripts/check-tool-versions.sh(wired in asmake check-tool-versions, a new prerequisite ofbuild/test/lint/package/release/coverage/coverage-html) which checks every one against what's actually installed and prints aWARNING:for each mismatch or missing tool.Per the ticket: every tool is checked before any warning is printed (no stopping at the first mismatch), and it never fails the build - a drift is a hint to investigate, not a hard gate. Also updated
setup-goreleaser/setup-gitleaksto install the exact pinned version instead of@latest.No
.gitlab-ci.ymlchange needed -make lint/make test/make packagealready depend on the new target, so CI gets this for free.See the plan comment on #27 for more detail.
requested review from @danny
assigned to @claude
Let's add shellcheck here too - needs to be at least v0.10.0 (as this is what would be installed in the CI image).
Shellcheck raises 'SC2329' for these functions ('function is never invoked'). This is informational only, and expected in this case. Ensure we add shellcheck excludes for these functions so that shellcheck output is clean.
It would be better if we could set ranges on these. E.g. go >= 1.26.0 would be fine in this project, but for other values we may want exact values (e.g. golangci-lint == 2.11.4).
Will require some changes to the check-tool-versions.sh script too.
Conceptually all Ok, but would like to go a bit further, as per comments.
requested changes
mentioned in commit
5fd8c5811cchanged this line in version 2 of the diff
changed this line in version 2 of the diff
added 1 commit
5fd8c581- Address review: version operators, shellcheck (#27)Compare with previous version
Added:
shellcheck >= 0.10.0in .tool-versions, wired intomake lint(shellcheck scripts/*), and added to .gitlab-ci.yml's defensive apt-get install list alongside yamllint/iptables.Added a
# shellcheck disable=SC2329directly above each installed_version_ function (per-function, not a blanket file-level disable) -shellcheck scripts/*is clean now..tool-versions lines are now
<tool> <op> <version>with op one of ==, >=, >, <=, < - go and shellcheck use >=, the other four keep == (matching that a patch difference in those has caused real problems before). check-tool-versions.sh's new version_compare/version_satisfies do numeric comparison via GNU sort -V (not a plain string match, which would get e.g. 2.9 vs 2.10 backwards).resolved all threads
All three review comments addressed in
5fd8c58:shellcheck >= 0.10.0added to.tool-versions, wired intomake lint, and added to the CI defensive-install list.# shellcheck disable=SC2329directives added for the indirectly-invokedinstalled_version_<tool>functions -shellcheck scripts/*now clean..tool-versionslines now take an operator (==/>=/>/<=/<) -go/shellcheckuse>=, the rest keep==. Comparison is numeric (GNUsort -V), not lexical.make lint/make testboth pass clean, including the newshellcheckstep. Ready for another look.LGTM
approved this merge request
aborted the automatic merge because the source branch was updated. Learn more.
added 1 commit
08aa545e- Fix shellcheck 0.10.0 CI failure: suppress SC2317 too (#27)Compare with previous version
mentioned in commit
cb8f04b3e8mentioned in issue #27