(WIP) Add specific criteria for PR review
Created by: seancolsen
Before
The steps to review a core PR give the reviewer quite a bit of leeway to "form an opinion about the fix" without very much guidance on how people should be forming those opinions.
After
Developers can use the following template when reviewing a core PR
-
It's a worthy pursuit. -
It works as advertised. -
It makes sense for all users. -
It's unlikely to surprise users. -
New code is readable and makes sense. -
It doesn't appear to introduce security vulnerabilities. -
Acceptance criteria are either: (a) not stated, or (b) met. -
Significant new functionality is either: (a) not present, or (b) covered by new unit tests. -
New documentation is either: (a) not needed, or (b) provided within this PR, or (c) provided in a separate PR which already exists.
Notes
This PR would be somewhat of a policy change (not just a docs improvement), and as such, I would want approval from people like @totten @eileenmcnaughton @seamuslee001 (and maybe others — not sure what the protocol is for making decisions like this).
This was just an idea I had. I won't push it. Very much open to swiftly closing this PR if others are not interested in it!