Levandor

Mando MR Review Patterns

4 min read

Mando MR Review Patterns

All 21 of Andras’s MRs in the mando repo were mined (2026-07-13, read-only glab API) for reviewer feedback, and the recurring themes were distilled into a new “Recurring review feedback” section of /Volumes/bandi/coding/poc/mando/AGENTS.md (untracked). This note is the provenance + evidence record: the mining scope, the nine themes with their source MRs, the config-direction finding, and the new rules Andras dictated into the guide in the same session.

For Agents

Treat the nine themes as pre-review gates for ANY mando MR: run the list against your diff BEFORE assigning reviewers. The operational copy lives in AGENTS.md (“Recurring review feedback” section); this note preserves the evidence and MR references behind it.

Mining provenance

  • Scope: all 21 MRs authored by andras.lederer in the mando repo, !322 through !558 (17 merged, 2 closed, 2 open).
  • Method: read-only glab api calls, 2026-07-13.
  • Yield: 108 reviewer notes. By reviewer: krisztian.fekete1 87, gabor.nagy6 13, balazs.mracsko.alpiq 7.
  • Output: the “Recurring review feedback” section of AGENTS.md, plus the dictated rules below. The entire AGENTS.md was also purged of em dashes.

The nine themes

1. Zero unrelated changes in the diff

MRs !437, !474, !495, !548, !556. Revert incidental churn, drop unrelated commits. The diff must contain only the ticket’s changes.

2. Delete ALL unused code before review

The single most repeated complaint in the corpus; reviewer frustration quote in !345. Dead code, unused helpers, leftover scaffolding: all gone before the MR is assigned.

3. Failures are errors, never warnings

The heaviest policy theme: 15+ “should be error” comments in !481; the policy statement comes from balazs in !481.

  • Log failures with mando_core::error! carrying an error-code enum variant.
  • Fatal fails the execution.
  • No custom error message strings: the error’s to_string() IS the message; human-readable context goes in the message field.

4. Placement

MRs !322, !345, !556. Route helpers live next to the routes; mod rows grouped; util code goes in util; name modules for what they are.

5. Completeness before review

MRs !362, !437. Avoid “you forgot this” / “half done” comments. Mirror sibling configs (e.g. data groups) so a new thing is wired everywhere its siblings are.

6. Green pipeline + no conflicts before assigning review

MRs !345, !474, !558. Never assign reviewers on a red pipeline or a conflicted branch.

7. rustfmt every new file

MR !556 (gabor). Every new file must be rustfmt-clean. Scope fmt to the touched files; never bare cargo fmt (reformats ~64 legacy files).

8. Keep the Jira ticket in sync

MR !437. The ticket must describe what was actually implemented; update it when scope shifts during the MR.

9. Idioms

MRs !345, !556.

  • Import error types from mando_core; no thiserror:: path prefix.
  • No unwrap: parse at build time or return Result (!345).
  • Bind the service once instead of duplicating per-type match arms (!556).

Config-direction finding

Two reviewers in MR !556 steered NEW config toward config::Config over Envconfig. Direction of travel only: existing adapters keep envconfig; do not mass-migrate.

New dictated rules (added to AGENTS.md, same session)

Rules Andras dictated alongside the mined themes, now codified in the guide:

  1. Commits are title-only with NO scopes and no descriptions. The no-scope rule is reviewer-mandated (MR !322).
  2. Commit every ~3 tasks, in short form.
  3. Never use em dashes anywhere. The whole AGENTS.md was purged of them.
  4. Always subagent-driven development, without asking.

Graph View

Backlinks