Skip to content

v0.12.3: golangci-lint v2 upgrade#140

Merged
yimsk merged 3 commits intomainfrom
develop
Jan 11, 2026
Merged

v0.12.3: golangci-lint v2 upgrade#140
yimsk merged 3 commits intomainfrom
develop

Conversation

@yimsk
Copy link
Copy Markdown
Contributor

@yimsk yimsk commented Jan 11, 2026

Summary

  • Upgrade golangci-lint from v1.64.8 to v2.8.0
  • Migrate config to v2 format
  • Fix lint issues: noctx (6), staticcheck (6)

Changes

  • .golangci.yml: Migrated to v2 format
  • exec.Commandexec.CommandContext (6 files)
  • Switch statement refactoring (4 files)
  • Error message formatting fix

@claude
Copy link
Copy Markdown

claude Bot commented Jan 11, 2026

PR Review: golangci-lint v2 Upgrade

Summary

This PR successfully upgrades golangci-lint from v1.64.8 to v2.8.0, migrates the configuration to v2 format, and fixes all lint issues discovered by the new linters. The changes are well-executed and improve code quality.

Strengths

1. Configuration Migration

  • Clean migration from v1 to v2 config format
  • All linter settings properly preserved
  • Good organization with separate sections for linters, formatters, and exclusions

2. Security Improvements

The addition of noctx linter is excellent for security:

  • All 6 instances of exec.Command changed to exec.CommandContext prevent potential context leak issues
  • Uses context.Background() appropriately for these CLI/test scenarios
  • Files updated: internal/action/exec_with_header.go:70,167, internal/app/pty_test.go:18,25, internal/view/profile_selector.go:235, scripts/gen-imports/main.go:67

3. Code Quality Improvements

Switch statement refactoring improves readability in 4 files

4. Bug Fix

  • custom/s3/buckets/dao.go:221 - Fixed unnecessary .BaseResource qualification

Issues and Concerns

Critical: Context Cancellation Not Propagated

Issue: All command executions use context.Background() instead of propagating the parent context.

Why This Matters:

  • Commands won't be cancelled when the app shuts down
  • Long-running operations (SSH sessions, AWS SSO login) may hang on exit
  • Users may need to force-kill the application

Recommendation: Use parent context instead of context.Background() for better lifecycle management

Minor Issues:

  • Error message style change in chat_overlay.go:522 (now more idiomatic)
  • Config duplication in .golangci.yml formatters.exclusions section

Security Assessment

  • noctx linter prevents context-less commands
  • No new security vulnerabilities introduced
  • context.Background() usage reduces graceful shutdown capabilities

Recommendation

Approve with suggestions. The PR successfully achieves its goal of upgrading golangci-lint. However, I recommend addressing the context cancellation issue before merging to ensure proper application lifecycle management.

Action Items (Optional):

  1. High Priority: Review context usage in command execution
  2. Low Priority: Deduplicate formatter exclusions in config

Review performed by Claude Code

@yimsk yimsk merged commit 67b111d into main Jan 11, 2026
9 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.

1 participant