19 add linting rule to restrict cyclomatic complexity in methods#45
19 add linting rule to restrict cyclomatic complexity in methods#45
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces cyclomatic/cognitive complexity linting via golangci-lint and updates local git hooks to run formatting/linting checks automatically, with a small documentation touch-up and a temporary suppression for a complex CLI entrypoint.
Changes:
- Add
.golangci.ymlenablinggocycloandgocognitwith configured complexity thresholds. - Update
lefthook.ymlto rungoimports,go vet, andgolangci-linton commit, and adjust pre-push build/vet commands. - Add/expand GoDoc comments in
internal/debuginfoand suppress complexity linting forcmd/bingo-client’smain.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
lefthook.yml |
Enforces goimports/vet/lint in hooks; changes pre-push build and commit-msg command execution. |
internal/debuginfo/debug_info.go |
Adds GoDoc comments for interface methods/types. |
cmd/bingo-client/main.go |
Adds a temporary //nolint suppression for complexity linters on main(). |
.golangci.yml |
Configures complexity linters (gocyclo, gocognit) and thresholds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| imports: | ||
| glob: "*.go" | ||
| run: gofmt -w {staged_files} && git add {staged_files} | ||
| run: goimports -w {staged_files} && git add {staged_files} |
There was a problem hiding this comment.
goimports is now invoked unconditionally; if a contributor doesn't have it on their PATH, the pre-commit hook will fail. Consider restoring the presence check or documenting/installing goimports as a required dev dependency (e.g., via a just tools target or similar).
| run: goimports -w {staged_files} && git add {staged_files} | |
| run: | | |
| if command -v goimports >/dev/null 2>&1; then | |
| goimports -w {staged_files} | |
| git add {staged_files} | |
| else | |
| echo "warning: goimports not found in PATH; skipping Go import formatting. Please install goimports." | |
| fi |
There was a problem hiding this comment.
It should in fact loudly fail if you don't have it on your path. For now this was not a problem because we used the devcontainer but we'll need to address this at some point
| if command -v golangci-lint &> /dev/null; then | ||
| golangci-lint run --new-from-rev=HEAD ./... | ||
| fi | ||
| run: golangci-lint run --new-from-rev=HEAD ./... |
There was a problem hiding this comment.
golangci-lint is also invoked unconditionally now, which will cause pre-commit to fail on machines where it's not installed. If this is intended, it should be documented as a required tool; otherwise, consider re-adding the command -v golangci-lint guard (or using a pinned go run/toolchain-based invocation).
| run: golangci-lint run --new-from-rev=HEAD ./... | |
| run: if command -v golangci-lint >/dev/null 2>&1; then golangci-lint run --new-from-rev=HEAD ./...; else echo "golangci-lint not installed; skipping golangci-lint"; fi |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.