- Enhanced CONTRIBUTING.md with -short flag usage guidance - Updated LINTING.md: 22 issues baseline (down from 34) - Added MCP exclusion note for bd edit command in AGENTS.md Closes bd-iov0, bd-aec5439f, bd-fd8753d9
4.6 KiB
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 errorstx.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
$EDITORfor 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-functionsworks for some errcheck patternsexcludepatterns with regex don't match as expectedexclude-ruleswith 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:
- Disable specific linters (errcheck, revive) if the signal-to-noise ratio doesn't improve
- Use
//nolintdirectives sparingly for clear false positives - Investigate alternative linters with better exclusion support
- 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.