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

[Thomas Stüfe]

One question, do merges with master cause the Skara reviewer check to fail?

We try to encourage authors to merge often, especially before pushing.
Patches should be based on a reasonably fresh copy of master. I believe the
contribution guidelines state that too, and it is just good practice.

Cheers, Thomas

On Tue, Jun 18, 2024 at 6:00 PM Iris Clark <iris.clark at oracle.com> wrote:

> Hi.
>
> 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.
>
> I propose that reviews be automatically marked as stale when the PR is
> updated, and re-review be required before integration.  Stale reviews do
> not
> count towards the minimum number of reviews required for integration.  The
> final commit's "Reviewed-by" field will continue to list all reviewers,
> regardless of whether they evaluated an older version of the PR.  When a
> PR is
> updated, instead of simply noting that the PR applies to a particular
> version,
> the review will be noted as stale, indicating that the PR does not meet
> integration requirements.  This re-review requirement has been in effect
> for
> the OpenJFX repos [2] since October 2019, when OpenJFX moved to GitHub
> using
> the Skara tooling.
>
> I suggest that we adopt this policy two weeks hence, on Tue 3 July.
>
> Concerns?
>
> [0] https://openjdk.org/guide/#fixing-a-bug (step 12)
> [1] https://github.com/openjdk/jdk
> [2] https://github.com/openjdk/jfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/jdk-dev/attachments/20240624/1073b4d8/attachment.htm>