Skip to content

pull-requests.md

Latest commit

 

History

History
114 lines (92 loc) · 5.2 KB

pull-requests.md

File metadata and controls

114 lines (92 loc) · 5.2 KB
Raw

Pull requests

What a mergeable PR looks like: the checklist, what reviewers expect, and how milestones and issues are organized.

Before you open it

Work through the contributing workflow first — branch naming, conventional commits, and the size guideline (≤ ~500 changed lines where feasible). Draft PRs are welcome early; open one as soon as you want CI feedback or design discussion.

Checklist

Every PR:

  • Builds and tests pass on all three OSes — the CI matrix (macOS, Linux, Windows) is the gate; no "fix Windows later" merges.
  • C++ is formatted (git clang-format; the format CI job enforces it).
  • Tests are included with the change, following the testing doctrine.

When applicable:

  • User-visible change (feature, behavior, CLI/API, packaging) → add a one-line entry under [Unreleased] in CHANGELOG.md, in the same PR, linked to its issue/PR.
  • Touched the OpenDRIVE parser/writer or added an xodr feature → fuzz corpus in core/tests/fuzz/corpus/ extended (see Testing).
  • Changed the public kernel API → python/src/bindings.cpp updated and at least one example in python/examples/ added or updated, same PR.
  • Added/updated a dependency → follow the dependency policy, including its THIRD_PARTY_LICENSES.md row in the same commit.
  • Added/updated a bundled asset → follow the asset policy, including its ASSETS_LICENSES.md row (CI runs the license check).
  • Touched geometry or parsing → ran the sanitizer build locally.
  • Added a Qt item model → its QAbstractItemModelTester test ships in the same commit (see Testing).

UI and interactive work: show it

PRs that change the editor UI or add interactive behavior (tools, panels, viewport interactions) must include screenshots or GIFs in the PR description. Reviewers should be able to judge the result without building the branch. Before/after pairs are ideal for visual changes.

Visual output: pixels are the acceptance test

Any PR touching mesh generation, materials, normals, or the renderer must include editor-rendered before/after screenshots in the PR description, and visual changes get maintainer approval before merge. Metric tests gate correctness; the maintainer gates appearance — the tee shipped with green metrics and a visibly wrong junction once, and this rule is why it will not happen again (see product parity — visual acceptance).

Capture screenshots with the editor's screenshot mode (works headless):

python scripts/editor_screenshot.py scene.xodr out.png --camera orbit

CI's editor visual artifacts job renders the canonical scenes (assets/samples/crossing.xodr, t_attach.xodr, overpass.xodr) and uploads the PNGs on every run — link or embed them when they cover your change.

Evidence lives in issues and PRs, never in the repo. Diagnostic renders, .glb dumps, and before/after images are attached to the tracking issue or the PR description; binary evidence directories are gitignored and must not be committed.

Review process

  • A Robomous maintainer reviews every PR; maintainer approval plus green CI are the merge requirements.
  • CI is the objective gate — reviewers will not merge around a red job, and you should not ask them to.
  • Review feedback focuses on correctness (geometric and standards correctness above all), the architecture boundaries, and test quality.
  • Force-pushing to address review is fine; the merge to main is what matters.

The project board, pillars, issues, and labels

  • The public project board is the task manager for RoadMaker: every tracked issue lives on it, grouped by pillar and status. Start there to see what is in flight and what comes next.
  • Work items are GitHub issues, grouped by the eight capability pillars of the roadmap (P1 Interaction & Navigation … P8 Scenarios). GitHub milestones mirror the pillars one-to-one and are pure progress trackers; there are no version milestones and no version labels — the roadmap has a single release at the end (release philosophy).
  • Each pillar has an epic-labeled issue listing its sprint issues; pillar work carries a matching label (pillar:P1 … pillar:P8) and PRs reference their issue (Closes #N). Sprint issues are titled pN-sM: short description and state their Scope, Acceptance (the golden-workflow steps they unblock), and what they supersede.
  • Found a bug or want a feature that isn't tracked? Open an issue first for anything non-trivial — it is the cheapest place to agree on an approach. New issues get triaged onto the board.

After the merge

Squash or merge commits keep the conventional-commit style so the history stays readable and releasable. Release packaging happens automatically on v* tags — see CI.