Consolidate documentation: move maintainer docs to docs/, remove redundant files
- Move RELEASING.md and LINTING.md to docs/ (maintainer-only content) - Delete WORKFLOW.md (agent workflow content belongs in AGENTS.md) - Delete TEXT_FORMATS.md (technical details belong in ADVANCED.md) - Update all cross-references to point to new locations - Keep CLAUDE.md (required by Claude Code) Reduces root-level docs from 20 to 16 files with clearer organization. Amp-Thread-ID: https://ampcode.com/threads/T-fe1db4f3-16c6-4a79-8887-c7f4c1f11c43 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
99
docs/LINTING.md
Normal file
99
docs/LINTING.md
Normal file
@@ -0,0 +1,99 @@
|
||||
# Linting Policy
|
||||
|
||||
This document explains our approach to `golangci-lint` warnings in this codebase.
|
||||
|
||||
## Current Status
|
||||
|
||||
Running `golangci-lint run ./...` currently reports **34 issues** as of Oct 27, 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. The remaining issues represent the acceptable baseline that doesn't warrant fixing.
|
||||
|
||||
## Issue Breakdown
|
||||
|
||||
### errcheck (24 issues)
|
||||
|
||||
**Pattern**: Unchecked errors from `defer` cleanup operations
|
||||
**Status**: Intentional and idiomatic
|
||||
|
||||
Examples:
|
||||
```go
|
||||
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 (10 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**: G115 - Integer overflow conversion (1 issue)
|
||||
**Status**: False positive - bounded by max retry count
|
||||
|
||||
The exponential backoff calculation is bounded by a small retry counter, making overflow impossible in practice.
|
||||
|
||||
## 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 34 lint warnings. The code quality is high.
|
||||
|
||||
**For code review**: Focus on:
|
||||
- New issues introduced by changes (not the baseline 34)
|
||||
- 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.
|
||||
Reference in New Issue
Block a user