📝 docs: add ADR for staged-only Git hooks formatting
- Add ADR-0012 documenting the decision to format only staged Go files - Update ADR README.md with new entry - Document rationale, alternatives, and verification results - Include future considerations for monitoring and CI/CD integration Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
This commit is contained in:
125
adr/0012-git-hooks-staged-only-formatting.md
Normal file
125
adr/0012-git-hooks-staged-only-formatting.md
Normal file
@@ -0,0 +1,125 @@
|
||||
# 12. Git Hooks: Staged-Only Formatting
|
||||
|
||||
**Date:** 2026-04-05
|
||||
**Status:** Accepted
|
||||
**Authors:** DanceLessonsCoach Team
|
||||
|
||||
## Context
|
||||
|
||||
The DanceLessonsCoach project implemented Git hooks to automatically run `go fmt` and `go mod tidy` before commits. Initially, the `go fmt` hook was configured to format **all Go files** in the repository, regardless of their staged status.
|
||||
|
||||
During implementation review, concerns were raised about this approach:
|
||||
|
||||
1. **Scope**: Formatting all files could modify files not intended for the current commit
|
||||
2. **Control**: Developers might want to commit specific changes without auto-formatting unrelated files
|
||||
3. **Safety**: Risk of accidentally including unintended changes in commits
|
||||
4. **Performance**: Unnecessary processing of files not being committed
|
||||
|
||||
## Decision
|
||||
|
||||
Modify the Git pre-commit hook to format **only staged Go files** instead of all Go files in the repository.
|
||||
|
||||
### Implementation
|
||||
|
||||
```bash
|
||||
# Old approach (formats ALL Go files):
|
||||
GOFILES=$(find . -name '*.go' -not -path "./vendor/*" -not -path "./.git/*")
|
||||
|
||||
# New approach (formats only STAGED Go files):
|
||||
STAGED_GOFILES=$(git diff --cached --name-only --diff-filter=ACM | grep '\.go$')
|
||||
```
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
1. **Precision**: Only formats files explicitly staged for commit
|
||||
2. **Developer Control**: Respects the developer's intent about which files to include
|
||||
3. **Safety**: Eliminates risk of accidentally modifying/committing unstaged files
|
||||
4. **Performance**: Faster execution for large projects (only processes relevant files)
|
||||
5. **Predictability**: Behavior matches developer expectations
|
||||
|
||||
### Negative
|
||||
|
||||
1. **Partial Formatting**: Unstaged files remain unformatted (could lead to formatting drift)
|
||||
2. **Manual Intervention**: Developers must remember to format files before staging
|
||||
3. **Inconsistency Risk**: Different parts of codebase could have different formatting
|
||||
|
||||
### Mitigations
|
||||
|
||||
1. **Documentation**: Clear documentation explaining the staged-only behavior
|
||||
2. **Developer Training**: Educate team on the importance of staging all files needing formatting
|
||||
3. **Pre-push Hook**: Consider adding a pre-push hook that checks entire codebase formatting
|
||||
4. **CI/CD Check**: Maintain CI/CD formatting validation as a safety net
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
### Alternative 1: Format All Files (Original Approach)
|
||||
**Pros**: Ensures whole project consistency
|
||||
**Cons**: Lack of control, potential unintended changes
|
||||
**Rejected**: Due to developer preference for explicit control
|
||||
|
||||
### Alternative 2: Format Changed Files (Staged + Unstaged)
|
||||
```bash
|
||||
CHANGED_GOFILES=$(git diff --name-only | grep '\.go$')
|
||||
STAGED_GOFILES=$(git diff --cached --name-only | grep '\.go$')
|
||||
ALL_CHANGED=$(echo "$CHANGED_GOFILES\n$STAGED_GOFILES" | sort -u)
|
||||
```
|
||||
**Pros**: Catches more formatting issues
|
||||
**Cons**: Still modifies files not explicitly staged
|
||||
**Rejected**: Doesn't respect staging intent
|
||||
|
||||
### Alternative 3: No Auto-Formatting
|
||||
**Pros**: Maximum developer control
|
||||
**Cons**: Inconsistent formatting, manual burden
|
||||
**Rejected**: Loses automation benefits
|
||||
|
||||
## Verification
|
||||
|
||||
### Test Results
|
||||
|
||||
Created test scenario with:
|
||||
- `test_staged.go`: Staged file with poor formatting
|
||||
- `test_unstaged.go`: Unstaged file with poor formatting
|
||||
|
||||
**Results**:
|
||||
- ✅ Staged file was formatted by hook
|
||||
- ✅ Unstaged file remained unformatted
|
||||
- ✅ Hook completed successfully
|
||||
|
||||
### Command Output
|
||||
```bash
|
||||
$ git add test_staged.go
|
||||
$ git commit -m "test"
|
||||
Running pre-commit hooks...
|
||||
Running go mod tidy...
|
||||
Running go fmt on staged files...
|
||||
Pre-commit hooks completed successfully
|
||||
```
|
||||
|
||||
## Related Decisions
|
||||
|
||||
- [ADR-0003: Zerolog Logging](adr/0003-zerolog-logging.md) - Project quality standards
|
||||
- [ADR-0004: Interface-Based Design](adr/0004-interface-based-design.md) - Code organization
|
||||
- [ADR-0010: API v2 Feature Flag](adr/0010-api-v2-feature-flag.md) - Feature management
|
||||
|
||||
## Future Considerations
|
||||
|
||||
1. **Monitor Impact**: Track if staged-only formatting leads to formatting drift
|
||||
2. **Developer Feedback**: Gather team input on the new behavior
|
||||
3. **CI/CD Enhancement**: Consider adding formatting validation in CI/CD pipeline
|
||||
4. **Editor Integration**: Document recommended editor settings for auto-formatting
|
||||
|
||||
## Revision History
|
||||
|
||||
- **1.0 (2026-04-05)**: Initial decision
|
||||
- **1.1 (2026-04-05)**: Added verification results and alternatives analysis
|
||||
|
||||
## References
|
||||
|
||||
- [Git Hooks Documentation](https://git-scm.com/docs/githooks)
|
||||
- [Go Formatting Standards](https://golang.org/doc/effective_go.html#formatting)
|
||||
- [Commit Message Skill with Hooks](.vibe/skills/commit_message/SKILL.md)
|
||||
|
||||
**Approved by:** DanceLessonsCoach Team
|
||||
**Effective Date:** 2026-04-05
|
||||
@@ -70,6 +70,7 @@ Chosen option: "[Option 1]" because [justification]
|
||||
* [0009-hybrid-testing-approach.md](0009-hybrid-testing-approach.md) - Combine BDD and Swagger-based testing
|
||||
* [0010-api-v2-feature-flag.md](0010-api-v2-feature-flag.md) - API v2 implementation with feature flag control
|
||||
* [0011-validation-library-selection.md](0011-validation-library-selection.md) - Selection of go-playground/validator for input validation
|
||||
* [0012-git-hooks-staged-only-formatting.md](0012-git-hooks-staged-only-formatting.md) - Git hooks format only staged Go files
|
||||
|
||||
## How to Add a New ADR
|
||||
|
||||
|
||||
Reference in New Issue
Block a user