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.
3.7 KiB
3.7 KiB
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:
daysSinceLastPaymentnotd - Don't lie with names:
accountsnotaccountListif it isn't a list - Pronounceable, searchable names — avoid encodings like
a2dp,strName - One word per concept across the codebase: pick
fetch,get, orretrieve— not all three - Names should read like prose:
if user.isEligibleForRefund()notif 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
checkPasswordmust 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
InvoiceCalculatoroverBusinessLogic
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 —
replicaCountnotrc,service.portnotp - 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
falseor""that always needs overriding is not a useful default