a85c7dc0fb
New skill files: - clean-code.instructions.md — naming, functions, classes, error handling, formatting, DRY/KISS/SOLID, Helm YAML conventions - clean-architecture.instructions.md — dependency rule, layers, boundaries, SOLID foundation, Helm as outermost layer - helm.instructions.md — resource ownership, values hygiene, required vs defaults, umbrella chart pattern, two-file values layering, KISS principle, hook ordering, config files pattern, dependency caching, template testing Register all three in ai-root-instructions.md skills list and routing table. Remove .ai from .gitignore — .ai/ is the product in this repo and must be tracked; the apply.sh skip-by-marker mechanism prevents changes to other repos.
79 lines
3.7 KiB
Markdown
79 lines
3.7 KiB
Markdown
# Clean Code
|
||
|
||
Based on Robert C. Martin's *Clean Code*. Apply these principles to all code and configuration, including Helm charts, YAML, and infrastructure-as-code.
|
||
|
||
---
|
||
|
||
## Naming
|
||
|
||
- Use intention-revealing names: `daysSinceLastPayment` not `d`
|
||
- Don't lie with names: `accounts` not `accountList` if it isn't a list
|
||
- Pronounceable, searchable names — avoid encodings like `a2dp`, `strName`
|
||
- One word per concept across the codebase: pick `fetch`, `get`, or `retrieve` — not all three
|
||
- Names should read like prose: `if user.isEligibleForRefund()` not `if user.check()`
|
||
|
||
## Functions
|
||
|
||
- **Do one thing** — if you can extract a meaningful function from it, it's doing two things
|
||
- **One level of abstraction per function** — don't mix high-level orchestration with low-level detail in the same function
|
||
- Keep short — aim for under 20 lines; use extract method aggressively
|
||
- 0–2 arguments preferred; use a parameter object when more are needed
|
||
- **No side effects** — a function named `checkPassword` must not also start a session
|
||
- No flag arguments: `delete(file, true)` means the function already does two things — split it
|
||
|
||
## Classes
|
||
|
||
- **Single Responsibility Principle** — one reason to change
|
||
- Small and cohesive — methods should use the class's fields, not act as utilities
|
||
- Hide internals: public API simple, private implementation complex
|
||
- No god classes — prefer `InvoiceCalculator` over `BusinessLogic`
|
||
|
||
## Error Handling
|
||
|
||
- Throw exceptions, don't return error codes — let callers handle what they understand
|
||
- Descriptive messages: `throw new UnauthorizedAccess("Admin role required for this action")`
|
||
- Define custom exceptions close to where they're used
|
||
- Clean up resources: use try/finally or RAII patterns; never leak
|
||
|
||
## Comments
|
||
|
||
- Good code needs no comments — if you need to explain *what*, rename instead
|
||
- Comments explain *why*, not *what*: acceptable for non-obvious decisions, legal notices
|
||
- TODOs are temporary; remove before merging
|
||
- Never commit commented-out code
|
||
|
||
## Formatting
|
||
|
||
- Related code stays close — declarations near first use
|
||
- Consistent indentation throughout the codebase — agree once, automate with linting
|
||
- No deep nesting — if you're 4 levels deep, extract a function
|
||
- Team standard beats personal preference every time
|
||
|
||
## General Principles
|
||
|
||
- **DRY** — every piece of knowledge has one representation; duplication is a maintenance liability
|
||
- **KISS** — the simplest solution that works; complexity must earn its place
|
||
- **Boy Scout Rule** — leave the code cleaner than you found it; small improvements compound
|
||
- **SOLID** — especially SRP (above) and Open-Closed: open for extension, closed for modification
|
||
- **Boundaries** — wrap third-party code in adapters; don't let external APIs bleed through the codebase
|
||
|
||
## Testing
|
||
|
||
- Tests must be **FAST, Independent, Repeatable, Self-Validating**
|
||
- One concept per test — a test that asserts many things is a test that hides failures
|
||
- Keep test code as clean as production code — it will be maintained just as long
|
||
- Write code testable from day one; retrofitting testability is expensive
|
||
|
||
---
|
||
|
||
## In Helm Charts and YAML
|
||
|
||
The same principles apply to configuration:
|
||
|
||
- **Naming**: value keys should be intention-revealing — `replicaCount` not `rc`, `service.port` not `p`
|
||
- **Single responsibility**: one chart does one thing; don't bundle unrelated services
|
||
- **No magic values**: named values in `values.yaml`, never hardcoded in templates
|
||
- **DRY**: use template helpers (`_helpers.tpl`) to avoid repeating label blocks and name patterns
|
||
- **Comments**: explain *why* a value is set a certain way, not what it does — the YAML speaks for itself
|
||
- **Defaults that make sense**: a default of `false` or `""` that always needs overriding is not a useful default
|