Phase 3, week 22retrospective

Upstream patch

The patch must follow upstream style, include tests, and stay narrowly scoped to the reported problem.


Style is not optional here

Run clang-format on the lines you touched rather than on the file. Follow the conventions of the file you edited, not the ones you prefer. Two rules catch outsiders most often: no using namespace in headers, and comments that explain why rather than what. The coding standards are short and the reviewer has read them.

One problem, one diff

Put a preparatory refactor in its own commit and mark it as no functional change, then the fix. A reviewer reads a diff twice, once for correctness and once for style; a mixed diff forces both reads on every line. If the change runs past a few hundred lines, suspect the scope rather than the reviewer’s patience.

Tests in the same commit

The regression test from the previous node ships with the fix, in the same commit, so a bisect never finds a state where behaviour changed without a test beside it. If existing tests change output, update the checks and confirm each change is intended: a regenerated check file can hide a behaviour change inside mechanical noise.

Write the message

[RISCV] Fold a splat into the reduction seed, then a body naming the bug, the mechanism and the test. The prefix is how the project routes review. The body is what a future reader has instead of your memory.

Things that break

A refactor and a fix in one diff, which no one can review. A regenerated check file that swallows an unintended change. A test that runs only on your host because it needs a registered target and carries no REQUIRES line. And a user-visible change with no release note, which is how a fix becomes folklore.

Sources

A version records the documentation the claim was checked against, not a house pin. Confirm flags with --help on the release you installed.

Check yourself

Why put the regression test in the same commit as the fix?

What is the right way to handle a preparatory refactor?

A regenerated check file changes three unrelated tests. What do you do?

0 / 3 correct

← back to the map