mirror of
https://github.com/meshtastic/firmware.git
synced 2026-09-16 08:30:04 -04:00
Tom
e09b961f39
* docs(agents): exempt test headers from the two-line comment limit The one-or-two-line comment rule rests on "the diff and commit message carry the rationale". For a test that premise is false. A test outlives the PR that added it, and the next person to read it is reading it because it failed - months later, in someone else's change, with the original discussion out of reach. That reader has one decision to make: real regression, or an expectation that has gone stale? The assertions alone cannot answer it, so the justification has to live in the file. The new "Test comments" section requires three things of a test header - what is under test by symbol and file, why that behavior is required, and the specific regression that returns if the assertions are deleted or relaxed - and grants whatever length they need. Authoritative statement lives in that section of .github/copilot-instructions.md, anchored at #test-comments. AGENTS.md and CLAUDE.md link to it rather than restate it. The test/** entry in .coderabbit.yaml carries the one permitted copy, since a YAML instruction cannot follow a link. This documents existing practice rather than changing it: 57 of the 72 suites with a test_main.cpp already open with three or more comment lines, test/test_gps_fix_hold/test_main.cpp with eleven. The rule as written forbade all of them, and automated reviewers acted on it. Scoped to the single comment bullet, not to the whole "General Style" section. Unlike "Naming Conventions", that section also holds the logging tiers and the Throttle rule, which bind test code as hard as src/; a blanket preamble would have quietly exempted tests from millis() discipline as well. The exception is bounded. The CodeRabbit entry still flags narrative that carries no contract - debugging journey, changelog prose, restating what the assertions plainly do - and per-case comments that merely repeat the test name. The documentation-does-not-live-here rule is untouched. * docs(agents): cut the test-comment rule to its essential statements A section about comment length had no business running to 373 words. Canonical section down to 194: dropped the elaboration of why the commit message is unavailable, the aside about which bullet reviewers most often cut, the "not licence for narrative" preamble, and the closing flourish. What remains is what an agent has to act on - the premise that fails for tests, the three things a header states, the instruction to reject a shorten-this review comment, and the worked example. Pointer files carry scope, not argument. The AGENTS.md bullet is one sentence and a link, matching the Test naming pointer directly above it. The .coderabbit.yaml copy keeps its imperatives, since a YAML instruction cannot follow the link to find them, but loses the causal explanation it did not need in order to act.
76 lines
4.2 KiB
YAML
76 lines
4.2 KiB
YAML
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
|
|
# CodeRabbit config - see https://docs.coderabbit.ai/getting-started/yaml-configuration
|
|
language: en-US
|
|
|
|
reviews:
|
|
# chill = fewer nitpicks.
|
|
profile: chill
|
|
high_level_summary: true
|
|
poem: false
|
|
auto_review:
|
|
enabled: true
|
|
# Don't burn reviews on WIP. This repo opens lots of draft PRs; review on "ready".
|
|
drafts: false
|
|
# Review PRs regardless of target branch.
|
|
base_branches:
|
|
- .*
|
|
# Skip Renovate dependency updates - CI gates dependencies; we review for substance, not every bump.
|
|
ignore_usernames:
|
|
- renovate
|
|
- renovate[bot]
|
|
# Stop reviewing once a PR is closed.
|
|
abort_on_close: true
|
|
|
|
path_filters:
|
|
# Generated - don't review, just noise + token burn.
|
|
- "!src/mesh/generated/**"
|
|
- "!protobufs/**"
|
|
|
|
path_instructions:
|
|
- path: bin/config.d/**
|
|
instructions: >
|
|
meshtasticd configuration files. Bundled with meshtasticd Linux/MacOS packaging.
|
|
Ensure configurations include metadata found in other configs.
|
|
- path: test/**
|
|
instructions: >
|
|
Native C++ unit tests. The camelCase convention used in src/ does NOT apply here,
|
|
and this is deliberate, not drift. Two separate rules. Suite directories are
|
|
strictly test_[a-z0-9_]+, lowercase only. Test functions take a test_ prefix
|
|
followed by underscore-separated segments, and the case WITHIN a segment is free:
|
|
test_5byte_sequence_rejected and test_getRegion_returnsCorrectRegion_US are both
|
|
correct and both common. Only two things are forbidden for a test function -
|
|
dropping the test_ prefix, and collapsing the segments into a single camelCase
|
|
identifier. Rationale: bin/run-tests.sh matches suite verdict lines against
|
|
test_[a-z0-9_]+, so an uppercase suite directory is reported as missing and
|
|
downgrades the run to AMBER; RUN_TEST in test/TestUtil.h passes #func to Unity, so
|
|
the function name is the only attribution a CI failure carries. Do NOT raise
|
|
naming-convention comments on suite directories or test_* functions, and do NOT
|
|
flag a camelCase segment inside an otherwise underscore-separated test name.
|
|
Helpers and fixtures inside a suite do follow the normal src/ conventions.
|
|
Authoritative rule, which this entry mirrors: the "Test naming" section of
|
|
.github/copilot-instructions.md.
|
|
|
|
Comment length is a second deliberate exception. The one-or-two-line comment limit
|
|
does NOT apply to a test's header comment. The header states what is under test by
|
|
symbol and file, why that behavior is required, and the regression that returns if
|
|
the assertions are deleted or relaxed; that routinely runs past two lines and is
|
|
correct at whatever length it needs. Do NOT ask for a test header to be shortened,
|
|
condensed, or moved to the commit message, and do NOT flag it as a multi-paragraph
|
|
block comment. Still DO flag narrative that carries no contract - debugging journey,
|
|
changelog prose, restating what the assertions do - and per-case comments that
|
|
merely repeat the test name. Authoritative rule, which this entry mirrors: the
|
|
"Test comments" section of .github/copilot-instructions.md.
|
|
- path: "**/*.md"
|
|
instructions: >
|
|
Documentation does not live in this repo; it lives in
|
|
https://github.com/meshtastic/meshtastic. Flag any NEW .md file that documents a
|
|
feature, configuration surface, API, wire format, or design, and ask for it to be
|
|
opened against the docs repo instead. Flag any attempt to recreate a docs/
|
|
directory: it was deleted in #11488 and must not come back. Flag write-ups left in
|
|
the tree - investigation notes, mitigation plans, migration checklists, "how we got
|
|
here" narrative, summaries of what a change did - that content belongs in the PR
|
|
description and commit message. Documentation that does belong upstream must read
|
|
as a technical manual, not a novel: what it does, the settings in user terms, the
|
|
API or protocol a client speaks. No debugging journey, no rationale essays, no
|
|
changelog prose.
|