|
| 1 | +# Comment Standards - Future Work vs Incomplete Implementation |
| 2 | + |
| 3 | +**Purpose:** Distinguish between legitimate future work markers and incomplete placeholder implementations. Allows proper project planning while maintaining implementation quality standards. |
| 4 | + |
| 5 | +**When to use:** Any time you need to mark future work, document planned enhancements, or indicate areas for improvement without blocking commits. |
| 6 | + |
| 7 | +## Allowed Comment Markers |
| 8 | + |
| 9 | +**Rule: Use structured markers for legitimate future work that should NOT block commits.** |
| 10 | +Why: These represent planned enhancements, design decisions, or documented technical debt - not incomplete current implementations. |
| 11 | + |
| 12 | +### FUTURE: - Planned Enhancements |
| 13 | +Use for features planned for future releases that don't affect current functionality. |
| 14 | + |
| 15 | +```rust |
| 16 | +// FUTURE: Add support for HTTP/2 transport in v2.0 |
| 17 | +// FUTURE: Implement connection pooling for better performance |
| 18 | +// FUTURE: Add metrics collection for monitoring dashboard |
| 19 | +pub fn create_connection() -> Connection { |
| 20 | + // Current working implementation |
| 21 | + Connection::new() |
| 22 | +} |
| 23 | +``` |
| 24 | + |
| 25 | +### NOTE: - Design Documentation |
| 26 | +Use for documenting design decisions, architectural notes, or implementation context. |
| 27 | + |
| 28 | +```rust |
| 29 | +// NOTE: We use stdio transport here because it's most compatible across platforms |
| 30 | +// NOTE: Error handling follows the established pattern from the validation module |
| 31 | +// NOTE: Performance is acceptable for current use cases (<100 users) |
| 32 | +pub fn establish_transport() -> StdioTransport { |
| 33 | + StdioTransport::new() |
| 34 | +} |
| 35 | +``` |
| 36 | + |
| 37 | +### ENHANCEMENT: - Performance/Quality Improvements |
| 38 | +Use for identified optimizations that aren't critical for current functionality. |
| 39 | + |
| 40 | +```rust |
| 41 | +// ENHANCEMENT: Could optimize this with a HashMap for O(1) lookups instead of O(n) |
| 42 | +// ENHANCEMENT: Add caching layer to reduce database queries |
| 43 | +// ENHANCEMENT: Consider using async stream for large result sets |
| 44 | +pub fn find_users(&self, criteria: &SearchCriteria) -> Vec<User> { |
| 45 | + // Current working implementation that meets requirements |
| 46 | + self.users.iter().filter(|u| criteria.matches(u)).cloned().collect() |
| 47 | +} |
| 48 | +``` |
| 49 | + |
| 50 | +### PLANNED(#issue): - Tracked Future Work |
| 51 | +Use for work items tracked in GitHub issues. Include issue number for traceability. |
| 52 | + |
| 53 | +```rust |
| 54 | +// PLANNED(#125): Add user authentication middleware |
| 55 | +// PLANNED(#126): Implement rate limiting for API endpoints |
| 56 | +// PLANNED(#127): Add database migration system |
| 57 | +pub fn handle_request(&self, request: Request) -> Response { |
| 58 | + // Current implementation handles unauthenticated requests |
| 59 | + self.process_request(request) |
| 60 | +} |
| 61 | +``` |
| 62 | + |
| 63 | +### CONSIDER: - Alternative Approaches |
| 64 | +Use for documenting alternative implementation approaches that were considered. |
| 65 | + |
| 66 | +```rust |
| 67 | +// CONSIDER: Alternative approach using trait objects instead of generics |
| 68 | +// CONSIDER: Could use macro to reduce boilerplate in similar functions |
| 69 | +// CONSIDER: Async version for non-blocking operations |
| 70 | +pub fn process_sync<T: Processor>(&self, processor: T) -> Result<Output> { |
| 71 | + // Current synchronous implementation that works for our use case |
| 72 | + processor.process(&self.data) |
| 73 | +} |
| 74 | +``` |
| 75 | + |
| 76 | +## Blocked Comment Markers |
| 77 | + |
| 78 | +**Rule: These markers indicate incomplete implementations and MUST be resolved before commit.** |
| 79 | +Why: These suggest the current code doesn't work properly or is missing essential functionality. |
| 80 | + |
| 81 | +### Blocked Patterns (Pre-commit will reject): |
| 82 | +```rust |
| 83 | +// TODO: Implement this function |
| 84 | +// FIXME: This is broken and needs repair |
| 85 | +// XXX: Hack that needs proper solution |
| 86 | +// HACK: Temporary workaround |
| 87 | +// stub implementation |
| 88 | +// placeholder implementation |
| 89 | +// For now, just return placeholder |
| 90 | +// Not implemented yet |
| 91 | +// Will implement later |
| 92 | +``` |
| 93 | + |
| 94 | +## Implementation Guidelines |
| 95 | + |
| 96 | +### Format Requirements |
| 97 | +```rust |
| 98 | +// ✅ GOOD - Structured with clear category |
| 99 | +// FUTURE: Add connection retry logic with exponential backoff |
| 100 | +// ENHANCEMENT: Optimize memory usage by reusing allocations |
| 101 | +// NOTE: This approach was chosen for compatibility with legacy systems |
| 102 | + |
| 103 | +// ❌ BAD - Vague or indicates incomplete work |
| 104 | +// TODO: Fix this |
| 105 | +// TODO: Make this work properly |
| 106 | +// TODO: Implement error handling |
| 107 | +``` |
| 108 | + |
| 109 | +### Linking to Issues |
| 110 | +```rust |
| 111 | +// ✅ GOOD - Links to tracked work |
| 112 | +// PLANNED(#123): Implement user roles and permissions system |
| 113 | +// FUTURE(#124): Add support for custom validation rules |
| 114 | + |
| 115 | +// ✅ GOOD - Internal planning without external tracking |
| 116 | +// FUTURE: Consider adding audit logging for compliance requirements |
| 117 | +// ENHANCEMENT: Profile this function to identify optimization opportunities |
| 118 | +``` |
| 119 | + |
| 120 | +### Context Requirements |
| 121 | +All future work markers should include sufficient context: |
| 122 | + |
| 123 | +```rust |
| 124 | +// ✅ GOOD - Explains what, why, and when |
| 125 | +// FUTURE: Add WebSocket transport support for real-time notifications |
| 126 | +// Planned for v2.0 when we have real-time requirements |
| 127 | +// Will require protocol extension for bidirectional communication |
| 128 | + |
| 129 | +// ❌ BAD - Lacks context |
| 130 | +// FUTURE: Add WebSocket support |
| 131 | +``` |
| 132 | + |
| 133 | +## Pre-commit Hook Configuration |
| 134 | + |
| 135 | +The pre-commit hook should be updated to: |
| 136 | + |
| 137 | +### Block These Patterns: |
| 138 | +- `TODO:` (without issue number) |
| 139 | +- `FIXME:` |
| 140 | +- `XXX:` |
| 141 | +- `HACK:` |
| 142 | +- `stub implementation` |
| 143 | +- `placeholder implementation` |
| 144 | +- `Not implemented` |
| 145 | +- `Will implement later` |
| 146 | +- Functions returning `unimplemented!()` or `todo!()` |
| 147 | + |
| 148 | +### Allow These Patterns: |
| 149 | +- `FUTURE:` |
| 150 | +- `NOTE:` |
| 151 | +- `ENHANCEMENT:` |
| 152 | +- `PLANNED(#\d+):` |
| 153 | +- `CONSIDER:` |
| 154 | +- `TODO(#\d+):` (when linked to issue) |
| 155 | + |
| 156 | +## Documentation Integration |
| 157 | + |
| 158 | +### Generate Future Work Reports |
| 159 | +```bash |
| 160 | +# Extract all future work markers |
| 161 | +grep -r "FUTURE:\|PLANNED:\|ENHANCEMENT:" src/ --include="*.rs" > future_work.md |
| 162 | + |
| 163 | +# Generate by category |
| 164 | +grep -r "FUTURE:" src/ --include="*.rs" | sed 's/.*FUTURE:/FUTURE:/' > planned_features.md |
| 165 | +``` |
| 166 | + |
| 167 | +### Review Process |
| 168 | +```markdown |
| 169 | +## Code Review Checklist |
| 170 | + |
| 171 | +### Future Work Markers |
| 172 | +- [ ] All TODO comments are either resolved or converted to appropriate markers |
| 173 | +- [ ] PLANNED items reference actual GitHub issues |
| 174 | +- [ ] FUTURE items have sufficient context for future developers |
| 175 | +- [ ] ENHANCEMENT items are genuine improvements, not missing functionality |
| 176 | +- [ ] NOTE items document important design decisions |
| 177 | +``` |
| 178 | + |
| 179 | +## Migration Guide |
| 180 | + |
| 181 | +### Converting Existing TODOs |
| 182 | +```bash |
| 183 | +# 1. Review all existing TODOs |
| 184 | +grep -r "TODO:" src/ --include="*.rs" |
| 185 | + |
| 186 | +# 2. Categorize each one: |
| 187 | +# - Is this missing functionality? → Fix before commit |
| 188 | +# - Is this planned future work? → Convert to FUTURE: or PLANNED(#issue): |
| 189 | +# - Is this a known optimization? → Convert to ENHANCEMENT: |
| 190 | +# - Is this documentation? → Convert to NOTE: |
| 191 | + |
| 192 | +# 3. Update pre-commit hook to use new patterns |
| 193 | +``` |
| 194 | + |
| 195 | +### Example Conversion |
| 196 | +```rust |
| 197 | +// OLD - Would be blocked |
| 198 | +// TODO: Add connection pooling |
| 199 | + |
| 200 | +// NEW - Allowed patterns |
| 201 | +// ENHANCEMENT: Add connection pooling to improve performance under high load |
| 202 | +// PLANNED(#145): Implement connection pooling for production scalability |
| 203 | +// FUTURE: Consider connection pooling when user base grows beyond 1000 concurrent users |
| 204 | +``` |
| 205 | + |
| 206 | +This system allows legitimate future work documentation while maintaining strict standards for current implementation completeness. |
0 commit comments