393 lines
21 KiB
Markdown
393 lines
21 KiB
Markdown
---
|
|
name: coding-dev
|
|
description: >-
|
|
Use this skill whenever writing, reviewing, refactoring, building, testing, or releasing code
|
|
in any language. Defines standing preferences on strictness, dependency choice, file and function
|
|
size limits, test discipline, build output handling, error/warning policy, and commit hygiene.
|
|
Links to per-language rules under code-languages/ for Rust, TypeScript/JavaScript, Bash, Python,
|
|
PHP, Go, C#/.NET, and C/C++.
|
|
---
|
|
|
|
# Coding & Development — Standing Rules and Preferences
|
|
|
|
These are the defaults for **all** code work, in any language. They are preferences with
|
|
reasons attached, not style trivia — most exist because ignoring them caused a real problem.
|
|
|
|
> [!IMPORTANT]
|
|
> **Whose rules win depends on where the code is going.** Always read a repository's
|
|
> `AGENTS.md`, `CLAUDE.md`, `AGENT.md`, `.rules`, or `CONTRIBUTING.md` first — but what you do
|
|
> on a conflict differs:
|
|
>
|
|
> | Situation | Precedence |
|
|
> | :--- | :--- |
|
|
> | **Contributing upstream** — a PR, a patch, or anything landing in someone else's repo | **The project's rules win.** Match their conventions exactly, even where they contradict this document. A PR that fights the house style gets rejected on style instead of reviewed on substance. |
|
|
> | **Local / private work** — our own repos, forks we don't upstream, private branches, local patches | **These rules win.** A project's conventions are a data point, not an obligation. Adopt the ones that are load-bearing (their build gate, their architecture contracts, anything that breaks if ignored) and apply our own preferences for the rest. |
|
|
>
|
|
> When in doubt about which mode you are in, **ask** — it changes the answer on strictness,
|
|
> file size, typing, and test policy all at once.
|
|
|
|
> [!IMPORTANT]
|
|
> **A new software idea starts with research, not code.** When the user describes a tool, app,
|
|
> service, or other project they want built, follow
|
|
> [`software-vision.md`](skills-public/software-vision.md) first: find existing projects to use,
|
|
> extend, or fork (web searches and user reviews first, then training-data recall, then
|
|
> throwaway clones), and present the options before scaffolding anything — unless the user
|
|
> explicitly opted out of research.
|
|
|
|
Per-language specifics live in [`code-languages/`](skills-public/code-languages/):
|
|
|
|
| Language | Rules |
|
|
| :--- | :--- |
|
|
| Rust | [`code-languages/rust.md`](skills-public/code-languages/rust.md) |
|
|
| TypeScript / JavaScript | [`code-languages/js-typescript.md`](skills-public/code-languages/js-typescript.md) |
|
|
| Bash / Shell | [`code-languages/bash.md`](skills-public/code-languages/bash.md) |
|
|
| Python | [`code-languages/python.md`](skills-public/code-languages/python.md) |
|
|
| PHP | [`code-languages/php.md`](skills-public/code-languages/php.md) |
|
|
| Go | [`code-languages/go.md`](skills-public/code-languages/go.md) |
|
|
| C# / .NET | [`code-languages/csharp-dotnet.md`](skills-public/code-languages/csharp-dotnet.md) |
|
|
| C / C++ | [`code-languages/c-cpp.md`](skills-public/code-languages/c-cpp.md) |
|
|
| Lua | [`code-languages/lua.md`](skills-public/code-languages/lua.md) |
|
|
| GSC (Call of Duty script) | [`code-languages/gsc.md`](skills-public/code-languages/gsc.md) |
|
|
|
|
---
|
|
|
|
## 1. Strictness — Maximum, Always
|
|
|
|
**Configure every linter, compiler, and type checker as strictly as the toolchain allows.**
|
|
Strict mode is the starting point, not an aspiration to migrate toward later.
|
|
|
|
- Warnings are errors in CI and in the build gate (`-warnaserror`, `-D warnings`, `--strict`).
|
|
- Enable the pedantic/extra lint tiers, not just the defaults.
|
|
- Type checking must pass with **zero errors or warnings** — `tsc --noEmit`, `mypy`,
|
|
`cargo check` alike.
|
|
- Never weaken a check to make a build pass. Fix the failing step. If a check is genuinely
|
|
wrong for a specific piece of code, **suppress it at the narrowest scope that covers the
|
|
problem — the line, the function, or the module — with a comment explaining why.** A
|
|
project-wide disable is a bigger hammer than it looks: it silences the rule everywhere,
|
|
including in code written later by someone who never saw the reason. Reach for it only when
|
|
the rule is genuinely wrong for the *whole* codebase, never as a shortcut for a stubborn
|
|
call site.
|
|
|
|
### Suppressions are a last resort, and they expire
|
|
|
|
- Try hard to actually fix an error or warning before suppressing it.
|
|
- When you do suppress one, **say so explicitly in your report to the user** — name the
|
|
warning, where it is, and why it could not be fixed. A silent suppression is the failure
|
|
mode this rule exists to prevent.
|
|
- **Every suppression carries a comment saying why.** A bare `#[allow]` / `// eslint-disable`
|
|
/ `# noqa` with no reason is indistinguishable from someone giving up, and nobody will ever
|
|
dare remove it.
|
|
- **Name the specific rule**, never a blanket suppression: `# noqa: S603`, not `# noqa`;
|
|
`//nolint:errcheck`, not `//nolint`.
|
|
- **Re-test suppressions periodically.** Removing one that no longer suppresses anything
|
|
should produce no warning — if it doesn't, delete it. Stale suppressions accumulate and
|
|
hide real regressions.
|
|
|
|
---
|
|
|
|
## 2. Never Hide Build, Test, or Lint Output
|
|
|
|
**Do not pipe build, compile, lint, format, or test output through `grep`, `tail`, `head`,
|
|
`sed`, or any other filter.** Run the command so its complete output is visible.
|
|
|
|
**Why:** filtering hides warnings, notes, and diagnostics that did not match the pattern.
|
|
Grepping for `^warning|^error` drops multi-line notes, help text, deprecation notices, and
|
|
anything a tool emits in an unexpected format.
|
|
|
|
```bash
|
|
# WRONG — silently drops notes, helps, and deprecations
|
|
cargo clippy 2>&1 | grep -E "^(warning|error)"
|
|
|
|
# RIGHT
|
|
cargo clippy --workspace --all-targets --all-features
|
|
```
|
|
|
|
If output is genuinely too large, **narrow the command** (one test, one package, one target)
|
|
rather than filtering its output. The same applies to logging: do not swallow stderr, do not
|
|
redirect diagnostics to `/dev/null`, and do not set a log level that hides warnings during a
|
|
verification run.
|
|
|
|
---
|
|
|
|
## 3. Dependencies — Reuse Before You Build
|
|
|
|
A strict ordering for obtaining any non-trivial functionality:
|
|
|
|
1. **Use an existing library / crate / package / official SDK** that already does it.
|
|
2. **If one is close but not exactly right, fork it** (as a git submodule or vendored fork)
|
|
and modify it.
|
|
3. **Write your own implementation only when nothing else fits** — and say why, explicitly.
|
|
|
|
**Why:** the ecosystem has already solved correctness, edge cases, and platform quirks that
|
|
a fresh implementation will rediscover slowly and incompletely.
|
|
|
|
Before hand-rolling any subsystem — a protocol client, a parser, a date formatter, a
|
|
debounce, a retry/backoff, a semver comparator — **search first and state the finding.**
|
|
Prefer official/first-party SDKs over third-party wrappers. The same ordering applies to a
|
|
whole project idea, not just a subsystem — see
|
|
[`software-vision.md`](skills-public/software-vision.md).
|
|
|
|
### The corollary: new generic code becomes its own package
|
|
|
|
**When you implement something generic that no existing package provided — a parser or
|
|
reader for a file format, a protocol implementation, a client for an undocumented API —
|
|
it becomes its own package/crate/module in its own repository**, which the application then
|
|
depends on like any other dependency.
|
|
|
|
**Why:** it keeps the app's tree focused on the app, it forces a clean public API boundary,
|
|
and it gives back the library that was missing in the first place. Structure the split so
|
|
the reusable core has no UI, no framework, and no app-specific types — only the app shell
|
|
depends on those.
|
|
|
|
> [!TIP]
|
|
> Design for this from the start: put **all reusable logic in a core library** and keep the
|
|
> UI/CLI layer as presentation only. A core without GUI dependencies also builds and tests
|
|
> far faster, often without the app's heavyweight toolchain.
|
|
|
|
---
|
|
|
|
## 4. Size Limits — Files and Functions
|
|
|
|
| Unit | Limit |
|
|
| :--- | :--- |
|
|
| **Source file** | **1000 lines** max |
|
|
| **Function** | **200 lines** max |
|
|
| **Function parameters** | ~5, then pass a struct/object |
|
|
| **Nesting depth** | 3, then extract |
|
|
|
|
These are the limits, not targets to stay well under: there is no earlier "split at" point.
|
|
A file of 700 lines with one coherent responsibility is fine and is not split for its size.
|
|
|
|
### Enforce both mechanically, never by eye
|
|
|
|
A limit nobody checks is a suggestion. Pick the **highest** applicable option:
|
|
|
|
1. **A linter rule**, where one exists — the function limit is usually covered natively
|
|
(Clippy's `too_many_lines` with `too-many-lines-threshold = 200`, ESLint's
|
|
`max-lines-per-function`, `golangci-lint`'s `funlen`), configured to 200 lines.
|
|
A function that trips it wants extracting, not a suppression. Note that some linters only
|
|
offer *proxies* for length — Ruff's `PLR0915` counts statements, not lines — in which case
|
|
enable the proxy and put the real limit in a test.
|
|
2. **A test**, for anything the linter does not cover — file length almost always falls here.
|
|
A test is the best home for it: it runs with the suite everyone already runs, it needs no
|
|
extra tooling, it fails with a message naming the offending files, and it travels with the
|
|
repository rather than living in one machine's shell script.
|
|
3. **The build script**, only as a fallback when the project has no test suite to hang it on.
|
|
|
|
Whichever you use, it should **fail over the limit** and name the offending files.
|
|
|
|
```rust
|
|
#[test]
|
|
fn no_source_file_exceeds_limits() {
|
|
const LIMIT: usize = 1000;
|
|
let mut over = Vec::new();
|
|
|
|
for path in source_files() {
|
|
let lines = std::fs::read_to_string(&path).unwrap().lines().count();
|
|
if lines > LIMIT {
|
|
over.push(format!("{} — {lines} lines (limit {LIMIT})", path.display()));
|
|
}
|
|
}
|
|
|
|
assert!(over.is_empty(), "files over the size limit:\n{}", over.join("\n"));
|
|
}
|
|
```
|
|
|
|
> [!NOTE]
|
|
> A test runs with the suite, not on every compile. If you genuinely need the check on every
|
|
> build, most toolchains have a pre-build hook for it (Cargo's `build.rs`, an MSBuild target,
|
|
> a bundler plugin) — but that cost is paid on every incremental build forever, and the suite
|
|
> is usually soon enough. Reach for the hook only if oversized files keep landing anyway.
|
|
- **Split by concern, never by line count.** A large `strip.rs` becomes
|
|
`strip/fader.rs`, `strip/knobs.rs`, `strip/routing.rs` — never `strip1.rs` / `strip2.rs`.
|
|
- One coherent responsibility per module. A module that draws widgets should not also own
|
|
application state.
|
|
|
|
---
|
|
|
|
## 5. Tests
|
|
|
|
- **Write tests for every feature where testing is possible.** State plainly when something
|
|
is untestable (GUI, hardware, live third-party service) rather than quietly skipping it.
|
|
- **Every bug that was fixed gets a regression test.** A bug that broke once will break again;
|
|
the test is what makes the second time cheap. This is the highest-value test you can write.
|
|
- **A skipped test fails the gate.** Silently skipped tests are worse than missing ones —
|
|
they report green.
|
|
- Unit tests for logic; integration tests for protocol/boundary behavior.
|
|
- **Never test destructive operations against real data.** Use fixtures, temporary
|
|
directories, injected backends, and disposable test accounts. The user's real config,
|
|
data, devices, accounts, and files must never be used as scratch space.
|
|
- **Prove the test exercises the path under test.** A tool that silently falls back to a
|
|
default target when the requested one does not resolve will produce a passing measurement
|
|
of the wrong thing. Verify the wiring before trusting the result.
|
|
- **Test the value, not the presence of the key.** A check that counts entries which exist
|
|
will pass against a cleared or null value — and reads exactly like a working fix.
|
|
|
|
---
|
|
|
|
## 6. Verification — Before Claiming Anything Works
|
|
|
|
Run the **whole** gate, not the convenient part of it. A typical full gate:
|
|
|
|
> format check → lint (strict/pedantic) → type check → build → tests (with race/sanitizer
|
|
> where available) → docs build → size limits → smoke run
|
|
|
|
- **Prefer the project's build script** (`./scripts/build.sh`, `make check`, `./build.sh --lint`)
|
|
over hand-typed commands. It encodes steps you will otherwise forget, and it stays correct
|
|
when the toolchain moves.
|
|
- **Read the numbers, not just the verdict.** A suppression count that has grown is worth
|
|
noticing even when everything passes.
|
|
- **Confirm the binary you are testing contains the change.** A file existing proves a file
|
|
exists, not that it is current. Compare its mtime against the commit, or wait for the build
|
|
to actually report success — testing a stale binary reads exactly like "the fix didn't work".
|
|
- **A change to visual/UI code is not done until it has been looked at.** Build, run,
|
|
screenshot, compare.
|
|
- **State plainly what is stubbed.** A component that draws its frame but has no behavior is a
|
|
scaffold, not a finished component — say so rather than implying completion.
|
|
|
|
---
|
|
|
|
## 7. Architecture Preferences
|
|
|
|
- **Events over polling — always.** When there is an event, callback, channel, or watch API to
|
|
subscribe to, use it. Do **not** settle for a polling loop because a benchmark shows it
|
|
currently costs nothing; a measurement of 0% CPU does not make polling the right design. It
|
|
adds latency and can miss or coalesce events. If existing code polls where an event exists,
|
|
treat that as a finding worth fixing. Only poll when no event source exists — and say so.
|
|
- **Don't over-guard.** Check only conditions that can actually occur. A null check on a
|
|
non-optional value, a `try/catch` around code that cannot throw, or an optional-chain on a
|
|
type-guaranteed value is dead defensive code, not safety.
|
|
- **No abstraction for one or two call sites.** A factory used once is just a function call.
|
|
No class wrapping a single function, no state machine for a boolean, no
|
|
`Manager`/`Handler`/`Strategy`/`Provider` suffix unless multiple implementations exist.
|
|
- **Less code with the same behavior wins.** Between equivalent solutions, the shorter ships.
|
|
Three similar lines beat a premature helper.
|
|
- **Delete dead code, don't comment it out.** That is what version control is for.
|
|
- **Don't cache what a store already owns.** Read from the source of truth on demand; cache
|
|
only what has no source of truth.
|
|
- **A search-and-replace that matches nothing is a silent failure.** Assert the target string
|
|
exists before replacing, and verify the result afterward.
|
|
|
|
### Model domain values as types, not primitives
|
|
|
|
**A value with rules of its own gets a type of its own.** Passing it around as a bare string,
|
|
int, or dict throws those rules away at the first function boundary and makes every consumer
|
|
re-derive them.
|
|
|
|
| Value | Use | Not |
|
|
| :--- | :--- | :--- |
|
|
| URL | the platform's URL type (`yarl.URL`, `Uri`, `url::Url`, `URL`) | `str` + f-string assembly |
|
|
| Filesystem path | `Path` / `PathBuf` / `std::filesystem::path` | `str` + manual separators |
|
|
| Closed set of values | enum / union of literals | bare strings compared with `==` |
|
|
| Structured record | struct, dataclass, or typed record | `dict[str, Any]` / `map[string]any` |
|
|
| Quantity with a unit | a wrapper carrying the unit | a bare number and a naming convention |
|
|
| Identifier of a specific kind | a distinct newtype | a `str` that any other id also fits |
|
|
|
|
**Why:** the type is where the rules live. A URL type knows how to join a path without
|
|
producing a double slash, escapes query parameters, and cannot be concatenated into something
|
|
malformed. A path type knows the platform separator. An enum cannot hold a typo. Each one
|
|
turns a class of runtime bug into something the compiler or checker rejects — and makes the
|
|
signature say what it accepts instead of `str`.
|
|
|
|
- **Convert at the boundary.** Parse incoming strings into the real type once, at the edge,
|
|
and keep the type inward. Serialize back to a string only where the wire or the filesystem
|
|
actually demands one.
|
|
- **Don't reach for a wrapper where the primitive genuinely is the value** — a person's name
|
|
is a string. The test is whether the value has invariants, a format, or operations that
|
|
callers keep re-implementing.
|
|
- Retrofit opportunistically: convert what you touch rather than rewriting a stringly-typed
|
|
codebase in one pass.
|
|
|
|
---
|
|
|
|
## 8. Scope Discipline
|
|
|
|
- **Solve the problem at hand.** No refactors unless asked. No "while I was in there" cleanups.
|
|
- **Smallest diff that correctly solves it.**
|
|
- **Don't refactor code you weren't asked to touch**, and don't mix unrelated concerns in one
|
|
change — backend work and UI work land separately.
|
|
- When you notice adjacent work worth doing, **describe it** rather than absorbing it.
|
|
- **Check `git status --short` before editing** and preserve unrelated in-progress changes.
|
|
- **Verify a TODO before acting on it.** Task documents are leads, not facts — grep for the
|
|
symbol, read the function, check the history. Work has been redone from scratch because a
|
|
stale TODO claimed something was missing that had shipped hours earlier.
|
|
|
|
---
|
|
|
|
## 9. Comments & Naming
|
|
|
|
- **Comment the *why*, never the *what*.** "A gap here, matching the reference layout" is
|
|
useful; `// draw the button` is not.
|
|
- **Don't narrate changes in comments** ("changed this to fix X") — that is git's job.
|
|
- Keep existing comments; they carry context you do not have.
|
|
- **Natural human text in user-facing strings** — errors, descriptions, toasts. Sentence case,
|
|
ending in a period, no robotic phrasing.
|
|
- **Never hardcode** what a constant, config file, or generated table already defines —
|
|
endpoints, colors, class names, translated strings, magic identifiers.
|
|
- Names checked mechanically stay correct; names checked by memory drift. If a project has a
|
|
naming rule that matters (a one-letter difference between the app and its library, say),
|
|
**put it in the build gate**.
|
|
|
|
---
|
|
|
|
## 10. Errors, Logging & Security
|
|
|
|
- Use the language's structured error type; do not stringify errors as the primary mechanism.
|
|
- **Never swallow an error silently.** No empty catch blocks.
|
|
- Use a real logger with levels — `debug` for traces, `info` for lifecycle, `warn` for
|
|
recoverable, `error` for unrecoverable. Not bare `print`/`console.log`.
|
|
- **Never log secrets** — tokens, auth headers, passwords, message content, personal data.
|
|
- **Never commit secrets.** Reference an environment file or secret store; commit only an
|
|
`.env.example` with empty values.
|
|
- **Treat every input from outside the trust boundary as hostile** — validate type, length,
|
|
and content. Normalize and bounds-check any path before touching the filesystem with it.
|
|
Never pass an externally-derived string to a shell.
|
|
- **Be extremely careful automating against real third-party services.** Use conservative
|
|
rate limits, generous delays, retries with backoff, and abort fast when a response looks
|
|
wrong. Assume you are one mistake away from a ban or a real-world side effect.
|
|
|
|
---
|
|
|
|
## 11. Git & Commits
|
|
|
|
- **Conventional Commits**: `feat(scope): …`, `fix(translator): …`, `test: …`, `docs: …`.
|
|
- Short, imperative subject lines.
|
|
- **Commit incrementally as work lands.** Never leave a large batch of uncommitted work at
|
|
the end of a task.
|
|
- **Submodules**: commit and push the submodule first, then the updated pointer in the parent.
|
|
Never leave a submodule commit unpushed — the parent would reference a commit nobody can fetch.
|
|
- Match the repository's existing trailer and attribution conventions; some repositories
|
|
deliberately carry none.
|
|
|
|
---
|
|
|
|
## 12. Reporting Work Honestly
|
|
|
|
- If tests fail, **say so and show the output**. If a step was skipped, say that.
|
|
- If a warning was suppressed rather than fixed, **name it**.
|
|
- Distinguish verified from unverified: GUI behavior, packaging, hardware, and live
|
|
third-party interactions are usually unverified unless you actually exercised them.
|
|
- Do not claim a visual match without having compared the two images.
|
|
- **Prefer the boring explanation.** A typo, a stale binary, or a self-matching pattern is
|
|
far more likely than a toolchain or sandbox bug — and an environment explanation is more
|
|
flattering and arrives faster, which is exactly why it is the one to distrust.
|
|
|
|
---
|
|
|
|
## 13. Environment & Tooling
|
|
|
|
Full version in [`cli-tooling.md`](skills-public/cli-tooling.md); the essentials:
|
|
|
|
- **Read `<tool> --help` before using a CLI.** Do not guess flags, and do not write an ad-hoc
|
|
script when an existing tool already does the job.
|
|
- **Standalone scripts live in a dedicated, version-controlled scripts directory**, symlinked
|
|
into `PATH` — never written directly into `~/.local/bin` or another `PATH` directory.
|
|
- **Build inside the project's designated environment** (container, venv, toolchain manager)
|
|
rather than the host, when the project specifies one. Note that entering a container
|
|
typically resets the working directory — chain `cd` inside the same command string.
|
|
- **Don't saturate the machine.** A full release build that consumes every core makes the
|
|
workstation unusable. When something latency-sensitive is running, halve the job count and
|
|
`nice` the build.
|
|
- **Prefer long-running work in the background** over blocking wait loops. Never write
|
|
`until <check>; do sleep 2; done` — it hangs the terminal and has to be interrupted by hand.
|