ci: make lint a hard gate (fix findings, pin golangci-lint v1.64.8) - #27
Conversation
- hurl: interface{} -> any in PostJSON / ResponseErrOrDecodeJson (the
config's gofmt rewrite rule)
- probe: rename the unused request parameter in the docs handler to _
- hurl tests: check the deferred Drain error
- types test: avoid S1025 by formatting the Secret inside a larger string
(still verifies fmt masking)
https://claude.ai/code/session_01J46cm6MaKsVpw1YSXGg8eq
The repo is now lint-clean (golangci-lint v1.64.8, the v1 line that reads the existing v1-format config). Drop the advisory --issues-exit-code=0 so lint findings fail the job, install golangci-lint directly for a deterministic version, and run it on Go 1.22 to match the test job. https://claude.ai/code/session_01J46cm6MaKsVpw1YSXGg8eq
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08f03416ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.8 | ||
|
|
||
| - name: Lint | ||
| run: "$(go env GOPATH)/bin/golangci-lint run ./..." |
There was a problem hiding this comment.
Run golangci-lint without quoting the entire command
In .github/workflows/ci.yml's lint job, wrapping $(go env GOPATH)/bin/golangci-lint run ./... in double quotes makes the shell treat the whole string (including run ./...) as the executable path, which resolves to “No such file or directory” and causes the lint step to fail before linting actually runs. This breaks the intended hard-gate behavior for every CI run using this workflow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 0934115. You're right that the wrapping quotes made the shell treat <path> run ./... as one executable path; I now export PATH and invoke golangci-lint bare in a block-scalar step. There was also a second cause: golangci-lint v1.64.8 needs Go ≥ 1.23 to build, but the job pinned Go 1.22, so go install failed too — bumped the lint job to 1.23.
Generated by Claude Code
Two reasons the lint job failed: golangci-lint v1.64.8 requires Go >= 1.23 to build (the job used 1.22, so `go install` failed), and wrapping the whole command in quotes made the shell treat "<path> run ./..." as a single executable path. Bump the lint Go to 1.23 and invoke golangci-lint via PATH in a block-scalar run step. https://claude.ai/code/session_01J46cm6MaKsVpw1YSXGg8eq
golangci-lint v1.64.x fully inactivated exportloopref (deprecated since v1.60.2; the Go 1.22 loopvar change made it irrelevant). Enabling a fully-inactivated linter makes golangci-lint exit non-zero (code 7) even with no findings, which failed the lint job. Remove it. https://claude.ai/code/session_01J46cm6MaKsVpw1YSXGg8eq
Summary
Turns the advisory lint job into a hard gate (Phase 0 follow-through from the v1 plan).
Running golangci-lint with the repo's config surfaced only 7 findings — and notably no missing-package-comment issues (golangci-lint's default exclusions already suppress those, so the earlier concern was unfounded). Fixed all of them:
hurl/client.go,hurl/util.go:interface{}→any(the config's gofmt rewrite rule)probe/docs.go: unused request parameter →_hurl/client_test.go(×3): check the deferredDrainerrortypes_test.go: avoidgosimpleS1025 while still verifyingfmtmasks aSecretThen the CI lint job:
--issues-exit-code=0→ findings now fail the build.golangci.yamland supports modern Go), installed directly for a deterministic version that matches local verificationlint (advisory)→lintTest plan
golangci-lint run ./...(v1.64.8) → 0 issuesgofmt -l .clean,go vet ./...go test ./...→ all 14 packages passNote
.golangci.yamlstill uses the v1 config format (v1.64.8 prints harmless deprecation warnings forrun.skip-dirs/govet.check-shadowing). Migrating to the v2 format is only needed if/when the team moves to golangci-lint v2 — tracked indocs/v1-release-plan.md.🤖 Generated with Claude Code
Generated by Claude Code