Skip to content

Commit

Permalink
Review checklist: suggest checking insertions expected to be new. (gf…
Browse files Browse the repository at this point in the history
…x-rs#7245)

Suggest checking that PRs assert that insertions into sets or maps
expected to be adding new values didn't actually just replace some
existing value.

Bug gfx-rs#7048 and its several duplicates would have been caught sooner if
the insertion of the new spill temporary into the `spilled_composites`
table had asserted that there was no existing spill variable for that
expression.
  • Loading branch information
jimblandy authored Feb 28, 2025
1 parent d6ca412 commit 3297e9f
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions docs/review-checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ satisfying way to address in a more robust way.
- [ ] If your change iterates over a collection, did you ensure the
order of iteration was deterministic? Using `HashMap` and
`HashSet` is fine, as long as you don't iterate over it.
- [ ] If you insert elements into a set or map that you expect are not
already present, did you make an assertion about `insert`'s
return value?

### WGSL Extensions

Expand Down

0 comments on commit 3297e9f

Please sign in to comment.