Skip to content

Conversation

@jlaneve
Copy link
Owner

@jlaneve jlaneve commented Jul 15, 2025

Add pre-commit hooks to catch linting and formatting issues before CI

Changes

  • Add .pre-commit-config.yaml with Go-specific hooks
  • Add .golangci.yml configuration for enhanced linting
  • Update README.md with development setup instructions

Pre-commit Hooks Include

  • Go formatting: gofmt, goimports
  • Go linting: go vet, golangci-lint
  • Dependencies: go mod tidy
  • Build check: go build
  • General: trailing whitespace, file endings, YAML/JSON validation

Fixes #6

Generated with Claude Code

- Add .pre-commit-config.yaml with Go-specific hooks
- Add .golangci.yml configuration for enhanced linting
- Update README.md with development setup instructions
- Hooks include gofmt, goimports, go vet, golangci-lint, and general checks

Fixes #6

Co-authored-by: Julian LaNeve <jlaneve@users.noreply.github.com>
@claude
Copy link
Contributor

claude bot commented Jul 15, 2025

Claude finished @jlaneve's task —— View job


Pre-commit Hooks Configuration Review

I've analyzed the pre-commit hooks configuration and here's my comprehensive review:

Strengths

.golangci.yml:

  • Excellent linter selection: Good balance of essential linters (errcheck, govet, staticcheck) and code quality tools (gocritic, revive)
  • Proper maintenance: Correctly disables deprecated linters (deadcode, varcheck, structcheck)
  • Sensible configuration: Cyclomatic complexity threshold of 15 is appropriate, and test file exclusions are logical
  • Performance focus: gocritic includes performance checks which is valuable

.pre-commit-config.yaml:

  • Established repos: Uses well-maintained and trusted pre-commit repositories
  • Comprehensive coverage: Handles formatting, imports, dependencies, building, and linting
  • Version pinning: All hooks are pinned to specific versions for reproducibility
  • Logical flow: Formatting → imports → dependencies → vet → build → lint is a good order

README.md:

  • Clear setup instructions: Easy to follow pre-commit installation steps
  • Good documentation: Explains what each hook category does
  • Integration: Fits well with existing development workflow documentation

🔧 Recommendations for Enhancement

Security Considerations:

  • Missing gosec linter: Consider adding gosec to .golangci.yml for security vulnerability detection:
    linters:
      enable:
        - gosec        # Security vulnerability checker

Configuration Improvements:

  • Version updates: .pre-commit-config.yaml:19 uses golangci-lint v1.54.2 which could be updated to latest stable
  • Additional useful linters: Consider adding to .golangci.yml:
    - bodyclose      # Check HTTP response body is closed
    - nilerr         # Check nil error returns  
    - sqlclosecheck  # Check SQL rows/statements are closed

Potential Issues:

  • Build performance: go-build-mod in pre-commit may be slow for large projects - consider if this should run on every commit
  • Hook order: golangci-lint runs after go-build-mod, but some linting issues might prevent building

🏆 Code Quality Assessment

  • Best practices: ✅ Follows Go community standards
  • Maintainability: ✅ Well-organized and documented
  • Completeness: ✅ Covers all major areas (formatting, linting, building, general file checks)
  • Performance: ✅ Reasonable timeout and complexity settings
  • Security: ⚠️ Could be enhanced with gosec addition

📊 Impact Assessment

This PR will significantly improve code quality by:

  • Catching formatting and import issues before CI
  • Preventing common Go mistakes and inefficiencies
  • Ensuring consistent code style across contributors
  • Reducing CI failures and review iterations

Files analyzed:

  • .golangci.yml:1-59 - Comprehensive linting configuration
  • .pre-commit-config.yaml:1-39 - Well-structured hook pipeline
  • README.md:200-233 - Clear development setup documentation

Overall Assessment

Approved - This is a well-thought-out pre-commit configuration that follows best practices. The minor enhancements suggested above are optional improvements, not blockers.

Priority fixes: None - configuration is solid as-is
Nice-to-have improvements: Add gosec for security scanning, consider hook performance optimization


@jlaneve
Copy link
Owner Author

jlaneve commented Jul 15, 2025

@claude I don't like using golangci here, can we remove it for now? I've had lots of issues with it locally

@claude
Copy link
Contributor

claude bot commented Jul 15, 2025

Claude finished @jlaneve's task —— View job


✅ Removed golangci-lint from pre-commit hooks

I've successfully removed golangci-lint from the pre-commit configuration as requested:

Changes Made:

  • Removed golangci-lint hook from .pre-commit-config.yaml
  • Deleted .golangci.yml configuration file (no longer needed)
  • Updated README.md to remove golangci-lint references in:
    • Make targets description (make lint now says "Run basic linting")
    • Pre-commit hooks documentation (removed golangci-lint bullet point)

Remaining Pre-commit Hooks:

The configuration now includes these lightweight Go tools:

  • gofmt: Code formatting
  • goimports: Import organization
  • go mod tidy: Dependency management
  • go vet: Basic linting
  • go build: Compilation check
  • General checks: trailing whitespace, file endings, YAML/JSON validation

The pre-commit setup should now work more reliably without the golangci-lint issues you were experiencing locally.


- Remove golangci-lint hook from .pre-commit-config.yaml
- Delete .golangci.yml configuration file
- Update README.md to remove golangci-lint references

Co-authored-by: Julian LaNeve <jlaneve@users.noreply.github.com>
@jlaneve jlaneve merged commit 4f19e2d into main Jul 15, 2025
3 of 4 checks passed
@jlaneve jlaneve deleted the claude/issue-6-20250715_042435 branch July 15, 2025 04:43
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.

pre-commit hooks

2 participants