Skip to content

ci: make lint a hard gate (fix findings, pin golangci-lint v1.64.8) - #27

Merged
hadifarnoud merged 4 commits into
masterfrom
chore/enable-lint-gate
May 26, 2026
Merged

ci: make lint a hard gate (fix findings, pin golangci-lint v1.64.8)#27
hadifarnoud merged 4 commits into
masterfrom
chore/enable-lint-gate

Conversation

@hadifarnoud

Copy link
Copy Markdown
Member

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 deferred Drain error
  • types_test.go: avoid gosimple S1025 while still verifying fmt masks a Secret

Then the CI lint job:

  • Drops --issues-exit-code=0findings now fail the build
  • Pins golangci-lint v1.64.8 (the last v1 line; reads the existing v1-format .golangci.yaml and supports modern Go), installed directly for a deterministic version that matches local verification
  • Runs on Go 1.22 (matches the test job); renamed lint (advisory)lint

Test plan

  • golangci-lint run ./... (v1.64.8) → 0 issues
  • gofmt -l . clean, go vet ./...
  • go test ./... → all 14 packages pass

Note

.golangci.yaml still uses the v1 config format (v1.64.8 prints harmless deprecation warnings for run.skip-dirs / govet.check-shadowing). Migrating to the v2 format is only needed if/when the team moves to golangci-lint v2 — tracked in docs/v1-release-plan.md.

🤖 Generated with Claude Code


Generated by Claude Code

claude added 2 commits May 25, 2026 22:36
- 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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread .github/workflows/ci.yml Outdated
run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.8

- name: Lint
run: "$(go env GOPATH)/bin/golangci-lint run ./..."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

claude added 2 commits May 26, 2026 08:54
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
@hadifarnoud
hadifarnoud merged commit 519f2c4 into master May 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants