docs(Agents): review must trace changed control flow into its consumers (#27137)
This commit is contained in:
parent
d6c71a85c7
commit
727104c656
2 changed files with 7 additions and 1 deletions
|
|
@ -20,6 +20,10 @@ line, not as background reading.
|
|||
- Never take a claim as fact, neither the PR description's nor a comment's. Verify game-data
|
||||
claims (spell/creature/quest ids, mechanics) against the world DB, DBC data, or cited sources;
|
||||
verify "fixed in the latest push" against the current diff.
|
||||
- When a change touches control flow (early returns, new branches, removed guards), trace the
|
||||
states it can leave behind (null, empty, fall-through) into every consumer of them — including
|
||||
unchanged lines the new flow now reaches — and verify what runs when an acquire/attack/GetX
|
||||
call leaves a null result, not just what enables it.
|
||||
- Check the change is still needed against current `master`: the surrounding code may have moved,
|
||||
or another change may have landed the same fix.
|
||||
- On an existing PR, walk every discussion item one by one: what was raised, whether it was
|
||||
|
|
|
|||
|
|
@ -24,4 +24,6 @@ Every change gets the full review; go deeper the farther it can reach:
|
|||
PRs are expected to be tested in-game, which the reviewer cannot do. Never guess what the
|
||||
author already tested — ask them, and record the answer in the report. Then tell them what
|
||||
else to test — especially side effects they might not expect: a fix for X that also reaches Y
|
||||
means testing Y too. Invite them to ask questions if anything is unclear.
|
||||
means testing Y too. When the author's tested scenario matches the change's main path, probe
|
||||
the branches it doesn't take — regressions hide there. Invite them to ask questions if
|
||||
anything is unclear.
|
||||
|
|
|
|||
Loading…
Reference in a new issue