|
| 1 | +--- |
| 2 | +description: When doing code reviews |
| 3 | +alwaysApply: false |
| 4 | +--- |
| 5 | + |
| 6 | +# Code Review Standards |
| 7 | + |
| 8 | +This rule defines what feedback is actionable based on project priorities. Use this to |
| 9 | +focus reviews on issues that actually get addressed. |
| 10 | + |
| 11 | +## Core Philosophy |
| 12 | + |
| 13 | +We ship pragmatically. Fix critical bugs, ensure security, validate core behavior. |
| 14 | +Ignore theoretical edge cases, minor polish, and over-engineering. Trust runtime |
| 15 | +validation over compile-time perfection. |
| 16 | + |
| 17 | +## Skip These Issues |
| 18 | + |
| 19 | +Do not include them in reviews. |
| 20 | + |
| 21 | +### Transactions for Rare Race Conditions |
| 22 | + |
| 23 | +Only flag race conditions you can demonstrate happen in practice. Theoretical edge cases |
| 24 | +requiring contrived timing are accepted as-is. Added complexity for extremely rare |
| 25 | +scenarios is not justified. |
| 26 | + |
| 27 | +### Magic Numbers and String Constants |
| 28 | + |
| 29 | +Constants exist to stay DRY, not to avoid "magic strings." This isn't Java. If a value |
| 30 | +appears once and the meaning is clear from context, inline is better than indirection. |
| 31 | +Only flag repeated values that would require multiple updates to change. Extracting |
| 32 | +`METHOD_INITIALIZE = "initialize"` for a single use is cargo cult programming. |
| 33 | + |
| 34 | +### Accessibility Improvements |
| 35 | + |
| 36 | +ARIA labels, keyboard navigation, and screen reader support are not current project |
| 37 | +priorities. |
| 38 | + |
| 39 | +### Minor Type Safety Improvements |
| 40 | + |
| 41 | +When runtime checks handle edge cases correctly, compile-time type refinement is |
| 42 | +optional. TypeScript serves the code, not vice versa. Working code takes priority over |
| 43 | +perfect types. |
| 44 | + |
| 45 | +### Test Edge Cases |
| 46 | + |
| 47 | +Core behavior receives test coverage. Edge cases including malformed API responses, |
| 48 | +empty arrays, and boundary conditions are deferred unless they cause production issues. |
| 49 | +Target is 90% coverage, not 100%. Focus on user-facing functionality. |
| 50 | + |
| 51 | +### Performance Micro-Optimizations |
| 52 | + |
| 53 | +Optimize when metrics show actual problems. Skipping unnecessary queries, adding |
| 54 | +theoretical indexes, or batching operations for minor latency gains without profiling |
| 55 | +data is premature optimization. |
| 56 | + |
| 57 | +### Documentation Enhancements |
| 58 | + |
| 59 | +Core documentation exists. Troubleshooting sections, rate limit documentation, and |
| 60 | +explanatory comments are iterative improvements based on user feedback. Perfect |
| 61 | +documentation delays shipping. |
0 commit comments