-
Notifications
You must be signed in to change notification settings - Fork 208
Code Review Checklist
Dan Connolly edited this page Jun 1, 2021
·
14 revisions
WIP. See also Coding Style. An initial draft based on a new team member's emerging understanding.
We delegate several checks to continuous integration tools:
-
test
: unit testing -
lint
: style, type annotation checking
But some aspects of house style are not automated:
- Object Capability Discipline - minimizing, if not eliminating, ambient authority (#2160)
- release notes; each package may use one of two styles:
- CHANGELOG.md is auto-generated from conventional commits
- NEWS.md is manual
- care with interleaving points: "The hazard is that, when both writing and reading a program, especially when reading other people's programs, an explicit
await
is not salient enough for people to understand that there's a turn boundary there, and that arbitrary turns interleave at that point, invalidating stateful assumptions." -- erights Apr 2020 - exceptions are not for flow control (or "errors are not normal API" or something like that)
- use
unknown
rather thanany
in type declarations (or document justification forany
) SES-shim/issues/600
See also: code-style issue label.
todo:
- get review by @erights and @warner (per 11 Feb discussion)
- CONTRIBUTING.md : move to or cite from
- github PR template : move / copy to or cite from