Skip to content
This repository was archived by the owner on May 6, 2026. It is now read-only.

LINTING.md

Latest commit

 

History

History
116 lines (79 loc) · 4.55 KB

LINTING.md

File metadata and controls

116 lines (79 loc) · 4.55 KB
Raw

Linting Policy

This document explains our approach to golangci-lint warnings in this codebase.

Current Status

Running golangci-lint run ./... currently reports 22 issues as of Nov 6, 2025. These are not actual code quality problems - they are false positives or intentional patterns that reflect idiomatic Go practice.

Historical note: The count was ~200 before extensive cleanup in October 2025, reduced to 34 by Oct 27, and now 22 after internal/daemonrunner removal. The remaining issues represent the acceptable baseline that doesn't warrant fixing.

Issue Breakdown

errcheck (4 issues)

Pattern: Unchecked errors from defer cleanup operations Status: Intentional and idiomatic

Examples:

defer rows.Close()
defer tx.Rollback()
defer os.RemoveAll(tmpDir)  // in tests

Rationale: In Go, it's standard practice to ignore errors from deferred cleanup operations:

  • rows.Close() - closing already-consumed result sets rarely errors
  • tx.Rollback() - rollback on defer is a safety net; if commit succeeded, rollback is a no-op
  • Test cleanup - errors during test cleanup don't affect test outcomes

Fixing these would add noise without improving code quality. The critical cleanup operations (where errors matter) are already checked explicitly.

gosec (12 issues)

Pattern 1: G204 - Subprocess launched with variable (3 issues) Status: Intentional - launching editor and git commands with user-specified paths

Examples:

  • Launching $EDITOR for issue editing
  • Executing git commands
  • Running bd daemon binary

Pattern 2: G304 - File inclusion via variable (3 issues) Status: Intended feature - user-specified file paths for import/export

All file paths are either:

  • User-provided CLI arguments (expected for import/export commands)
  • Test fixtures in controlled test environments
  • Validated paths with security checks

Pattern 3: G301/G302/G306 - File permissions (3 issues) Status: Acceptable for user-facing database files

  • G301: 0755 for database directories (allows other users to read)
  • G302: 0644 for JSONL files (version controlled, needs to be readable)
  • G306: 0644 for new JSONL files (consistency with existing files)

Pattern 4: G201/G202 - SQL string formatting/concatenation (3 issues) Status: Safe - using placeholders and bounded queries

All SQL concatenation uses proper placeholders and is bounded by controlled input (issue ID lists).

misspell (3 issues)

Pattern: British vs American spelling - cancelled vs canceled Status: Acceptable spelling variation

The codebase uses "cancelled" (British spelling) in user-facing messages. Both spellings are correct.

unparam (4 issues)

Pattern: Function parameters or return values that are always the same Status: Interface compliance and future-proofing

These functions maintain consistent signatures for:

  • Interface implementations
  • Future extensibility
  • Code clarity and documentation

golangci-lint Configuration Challenges

We've attempted to configure .golangci.yml to exclude these false positives, but golangci-lint's exclusion mechanisms have proven challenging:

  • exclude-functions works for some errcheck patterns
  • exclude patterns with regex don't match as expected
  • exclude-rules with text matching doesn't work reliably

This appears to be a known limitation of golangci-lint's configuration system.

Recommendation

For contributors: Don't be alarmed by the 22 lint warnings. The code quality is high.

For code review: Focus on:

  • New issues introduced by changes (not the baseline 22)
  • Actual logic errors
  • Missing error checks on critical operations (file writes, database commits)
  • Security concerns beyond gosec's false positives

For CI/CD: The current GitHub Actions workflow runs linting but doesn't fail on these known issues. We may add --issues-exit-code=0 or configure the workflow to check for regressions only.

Future Work

Potential approaches to reduce noise:

  1. Disable specific linters (errcheck, revive) if the signal-to-noise ratio doesn't improve
  2. Use //nolint directives sparingly for clear false positives
  3. Investigate alternative linters with better exclusion support
  4. Contribute to golangci-lint to improve exclusion mechanisms

Summary

These "issues" are not technical debt - they represent intentional, idiomatic Go code. The codebase maintains high quality through:

  • Comprehensive test coverage (>80%)
  • Careful error handling where it matters
  • Security validation of user input
  • Clear documentation

Don't let the linter count distract from the actual code quality.