jared
cd1af67a3a
The exclusion patterns are substring matches, so `chore:` never matched `chore(packaging):`. Because the cadence rule in AGENTS.md mandates the `type(ws):` scope, every chore and docs commit was being reviewed -- job 15 spent a thorough review on a config-only commit. Adds the `chore(` and `docs(` forms. Also names pm/compass.toml as authoritative for the review rules, so the prose copy in AGENTS.md cannot silently drift from the generated .roborev.toml. Raised by roborev job 15.
65 lines
5.1 KiB
TOML
65 lines
5.1 KiB
TOML
# roborev configuration, initialised by compass.
|
|
# Reviews are queued to a background daemon -- they never block a commit.
|
|
|
|
post_commit_review = 'commit'
|
|
# Scoped conventional commits (`chore(packaging):`) do not contain `chore:`,
|
|
# and this repo's cadence rule mandates the scope -- so both forms are listed.
|
|
excluded_commit_patterns = ['WIP', 'chore:', 'chore(', 'docs:', 'docs(', 'Merge ']
|
|
|
|
review_guidelines = '''
|
|
# --- compass:begin (generated -- edit the sources, not this) ---
|
|
- Prefer returning new values to mutating arguments in place. A function that edits
|
|
its caller's object is a bug waiting for a second caller.
|
|
- Validate at system boundaries -- user input, API responses, file contents, config.
|
|
Fail fast with a message naming the field and the file.
|
|
- Never swallow an error. Handle it or let it propagate; a bare catch that continues
|
|
is worse than a crash.
|
|
- No hardcoded secrets, tokens, or credentials, and no secrets in log output or error
|
|
messages.
|
|
- Parameterise every query. String-built SQL is a defect even when the input looks safe.
|
|
- Keep functions under roughly 50 lines and files under roughly 400. Flag nesting
|
|
deeper than four levels.
|
|
- No magic numbers or hardcoded paths -- name them as constants or read them from config.
|
|
- New behaviour needs a test. A bug fix needs a test that fails without the fix.
|
|
- Prose a person reads -- an issue title or body, a journal entry, a decision record,
|
|
the narrative on the status board -- names the action or the thing, not the shape of
|
|
the machinery. Flag "gate", "seam", "surface area", "load-bearing", "first-class",
|
|
"primitive", "blast radius". A project's own defined vocabulary is not the target.
|
|
- Use the native pipe `|>`, not magrittr `%>%`.
|
|
- snake_case for objects and functions; UPPER_SNAKE for constants. Never use `.` as a
|
|
word separator in a function name -- it collides with S3 dispatch.
|
|
- Validate arguments at the top of exported functions with `stopifnot()` or an explicit
|
|
check, and say which argument was wrong.
|
|
- Never `setDT()`, `set()`, or otherwise modify by reference a data.table the caller
|
|
still owns. `as.data.table()` copies; use it.
|
|
- Prefer `vapply()` to `sapply()` -- `sapply()` silently returns a list when the type
|
|
varies, which turns a type error into a downstream mystery.
|
|
- Use `seq_len(n)` / `seq_along(x)`, never `1:n`, which iterates backwards when n is 0.
|
|
- Compare strings with `==` only after checking for NA; use `identical()` for scalars
|
|
where NA would be wrong.
|
|
- Do not call `library()` inside package or module files; attach packages in scripts and
|
|
test helpers only.
|
|
- Namespace-qualify calls into other packages (`stats::sd`) in code that is sourced.
|
|
- Every exported function needs roxygen with `@param` for each argument (type, meaning,
|
|
and why the default is what it is) and `@return`. Add `@examples` for exported API.
|
|
- Declare dependencies in DESCRIPTION. Prefer base R or an existing dependency over
|
|
adding a new one; a package with zero hard deps is worth keeping that way.
|
|
- Signal errors with `stop()` carrying a condition class, so callers can catch the kind
|
|
rather than matching on message text.
|
|
- Keep internals internal. Export only what a user needs; an accidentally exported
|
|
helper becomes an API you have to keep.
|
|
- Tests use testthat edition 3. Each test is self-sufficient -- no reliance on state
|
|
left by an earlier test or on a fixture built elsewhere in the file.
|
|
- Prefer duplication in tests over a helper that hides what is being asserted.
|
|
- Brand colors come from `civilytics_colors`; never write a hex literal in theme, scale, logo, or table code. `R/flextable.R` still carries off-brand Bootstrap defaults (#2c3e50, #f0f0eb, #888888, #cccccc, #555555) -- do not add more.
|
|
- No tidyverse dependency. Imports is base R plus ggplot2, grid/gridExtra, png/jpeg, stringr, stringdist, showtext/sysfonts, jsonlite. Reject dplyr, purrr, magrittr, tibble, and data.table; use base R idioms.
|
|
- NAMESPACE is roxygen-generated. Declare imports with `@importFrom` in the roxygen block and re-run roxygen; never hand-edit NAMESPACE.
|
|
- Themes default to a transparent background (`paper_bg = FALSE`) so plots composite onto any Quarto, Reveal, or Typst background. Making an opaque background the default is a regression, not a preference.
|
|
- Theme functions thread `ink`/`paper`/`accent` into ggplot2 4.0's base-theme parameters rather than setting element colors ad hoc. ggplot2 >= 4.0 is a hard dependency, so use S7 `@` property access on plot and theme objects, not `$`.
|
|
- User-facing progress goes through `message()` so callers can suppress it. No `cat()` or `print()` in package code.
|
|
- Nothing personal or client-identifying is vendored into `inst/` -- brand assets only. Version 0.3.2 removed headshots for exactly this reason.
|
|
- The camelCase exports (`countCleanr`, `dbSafeNames`, `simpleCap`, `waldInterval`, `countDots`, `countNA`, `findDots`, `nvals`) are frozen public API; do not rename them. New functions are snake_case.
|
|
- `R/theme.R` and `R/logo.R` are long by design -- one file per surface, with dense roxygen. File length is not a finding in this package; flag a single function over roughly 80 lines instead.
|
|
# --- compass:end ---
|
|
'''
|