-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Labels
Description
UpdatePoolMeta Has Confusing Code
Priority: P2 - Medium (Quick Win)
Estimate: 15 minutes
Review Reference: REVIEW.md (P2-3)
Issue
Suspicious pattern in UpdatePoolMeta:
File: internal/storage/store.go:104-106
if accountID != nil || true { // This condition is always true!
p.AccountID = accountID
}The || true makes the condition meaningless. This appears to be a workaround for setting accountID to nil, but the pattern is confusing.
Same issue in: internal/storage/sqlite/sqlite.go:144
Impact: Low - Code smell, but functionally works
Solution
Simply remove the condition since the intent is to always apply the update:
// Always apply accountID update (including clearing to nil)
p.AccountID = accountIDAcceptance Criteria
- Remove
|| truecondition in MemoryStore - Remove
|| truecondition in SQLite store - Add test for clearing accountID to nil
- Add comment explaining nil assignment is intentional
- All existing tests pass
Validation
make test
# Add specific test for nil accountID:
# - Create pool with accountID set
# - Update with accountID = nil
# - Verify accountID is clearedRelated
This is a quick win that improves code clarity.