Proposal: Require re-review before integration if the PR is modified

[Andrew Haley]

On 6/18/24 17:00, Iris Clark wrote:

 > Every PR must be reviewed by at least one Reviewer [0] before it's
 > integrated into the main-line repository [1]. While we recommend
 > that the PR contain the final version of the change at creation
 > time, this is not always feasible, particularly for large or complex
 > changes. The integrated version of the PR may evolve significantly
 > from when the review was done. Thus the final commit's "Reviewed-by"
 > field may give false confidence that the complete set of changes has
 > been reviewed before it was integrated into the repository. This
 > could, in the worst case, allow an adversary to commit malicious
 > code.

All true. However, it would mean that people (if I'm honest, me)
wouldn't make last-minute corrections to comments, etc. And it would
extend the latency for some patches.

There is a great deal to be said for reviewing complex patches in the
open, and starting early, because a PR author cannot predict what
objections they may face.

Maybe this policy will result in some otherwise unnecessary
re-reviewing. But as long as it's only a single reviewer to do a final
pass, rather than a tick from everyone who has approved, I agree with
this policy.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671