Document -short flag, update linting baseline, clarify bd edit MCP exclusion
- 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
This commit is contained in:
@@ -172,6 +172,8 @@ bd update <id> [<id>...] --status in_progress --json
|
|||||||
bd update <id> [<id>...] --priority 1 --json
|
bd update <id> [<id>...] --priority 1 --json
|
||||||
|
|
||||||
# Edit issue fields in $EDITOR (HUMANS ONLY - not for agents)
|
# Edit issue fields in $EDITOR (HUMANS ONLY - not for agents)
|
||||||
|
# NOTE: This command is intentionally NOT exposed via the MCP server
|
||||||
|
# Agents should use 'bd update' with field-specific parameters instead
|
||||||
bd edit <id> # Edit description
|
bd edit <id> # Edit description
|
||||||
bd edit <id> --title # Edit title
|
bd edit <id> --title # Edit title
|
||||||
bd edit <id> --design # Edit design notes
|
bd edit <id> --design # Edit design notes
|
||||||
|
|||||||
@@ -135,16 +135,28 @@ Slow tests use `testing.Short()` to skip when `-short` flag is present.
|
|||||||
### Running Tests
|
### Running Tests
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# Fast tests (recommended for development)
|
# Fast tests (recommended for development - skips slow tests)
|
||||||
|
# Use this for rapid iteration during development
|
||||||
go test -short ./...
|
go test -short ./...
|
||||||
|
|
||||||
# Full test suite (before committing)
|
# Full test suite (before committing - includes all tests)
|
||||||
|
# Run this before pushing to ensure nothing breaks
|
||||||
go test ./...
|
go test ./...
|
||||||
|
|
||||||
# With race detection and coverage
|
# With race detection and coverage
|
||||||
go test -race -coverprofile=coverage.out ./...
|
go test -race -coverprofile=coverage.out ./...
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**When to use `-short`:**
|
||||||
|
- During active development for fast feedback loops
|
||||||
|
- When making small changes that don't affect integration points
|
||||||
|
- When you want to quickly verify unit tests pass
|
||||||
|
|
||||||
|
**When to use full test suite:**
|
||||||
|
- Before committing and pushing changes
|
||||||
|
- After modifying git operations or multi-clone scenarios
|
||||||
|
- When preparing a pull request
|
||||||
|
|
||||||
### Writing Tests
|
### Writing Tests
|
||||||
|
|
||||||
- Write table-driven tests when testing multiple scenarios
|
- Write table-driven tests when testing multiple scenarios
|
||||||
|
|||||||
@@ -4,13 +4,13 @@ This document explains our approach to `golangci-lint` warnings in this codebase
|
|||||||
|
|
||||||
## Current Status
|
## 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.
|
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. The remaining issues represent the acceptable baseline that doesn't warrant fixing.
|
**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
|
## Issue Breakdown
|
||||||
|
|
||||||
### errcheck (24 issues)
|
### errcheck (4 issues)
|
||||||
|
|
||||||
**Pattern**: Unchecked errors from `defer` cleanup operations
|
**Pattern**: Unchecked errors from `defer` cleanup operations
|
||||||
**Status**: Intentional and idiomatic
|
**Status**: Intentional and idiomatic
|
||||||
@@ -29,7 +29,7 @@ defer os.RemoveAll(tmpDir) // in tests
|
|||||||
|
|
||||||
Fixing these would add noise without improving code quality. The critical cleanup operations (where errors matter) are already checked explicitly.
|
Fixing these would add noise without improving code quality. The critical cleanup operations (where errors matter) are already checked explicitly.
|
||||||
|
|
||||||
### gosec (10 issues)
|
### gosec (12 issues)
|
||||||
|
|
||||||
**Pattern 1**: G204 - Subprocess launched with variable (3 issues)
|
**Pattern 1**: G204 - Subprocess launched with variable (3 issues)
|
||||||
**Status**: Intentional - launching editor and git commands with user-specified paths
|
**Status**: Intentional - launching editor and git commands with user-specified paths
|
||||||
@@ -54,10 +54,27 @@ All file paths are either:
|
|||||||
- G302: 0644 for JSONL files (version controlled, needs to be readable)
|
- G302: 0644 for JSONL files (version controlled, needs to be readable)
|
||||||
- G306: 0644 for new JSONL files (consistency with existing files)
|
- G306: 0644 for new JSONL files (consistency with existing files)
|
||||||
|
|
||||||
**Pattern 4**: G115 - Integer overflow conversion (1 issue)
|
**Pattern 4**: G201/G202 - SQL string formatting/concatenation (3 issues)
|
||||||
**Status**: False positive - bounded by max retry count
|
**Status**: Safe - using placeholders and bounded queries
|
||||||
|
|
||||||
The exponential backoff calculation is bounded by a small retry counter, making overflow impossible in practice.
|
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
|
## golangci-lint Configuration Challenges
|
||||||
|
|
||||||
@@ -70,10 +87,10 @@ This appears to be a known limitation of golangci-lint's configuration system.
|
|||||||
|
|
||||||
## Recommendation
|
## Recommendation
|
||||||
|
|
||||||
**For contributors**: Don't be alarmed by the 34 lint warnings. The code quality is high.
|
**For contributors**: Don't be alarmed by the 22 lint warnings. The code quality is high.
|
||||||
|
|
||||||
**For code review**: Focus on:
|
**For code review**: Focus on:
|
||||||
- New issues introduced by changes (not the baseline 34)
|
- New issues introduced by changes (not the baseline 22)
|
||||||
- Actual logic errors
|
- Actual logic errors
|
||||||
- Missing error checks on critical operations (file writes, database commits)
|
- Missing error checks on critical operations (file writes, database commits)
|
||||||
- Security concerns beyond gosec's false positives
|
- Security concerns beyond gosec's false positives
|
||||||
|
|||||||
Reference in New Issue
Block a user