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

[Kevin Rushforth]

Yes, it will. At least the minimum number of reviewers will need to 
re-review.

-- Kevin


On 6/18/2024 10:27 AM, Daniel Fuchs wrote:
> Hi Iris,
>
> I would welcome this change. But would it block integration
> if the new changeset that was pushed is only a merge changeset,
> merging changes from the mainline into the PR for a last round
> of testing?
>
> best regards,
>
> -- daniel
>
> On 18/06/2024 17:00, Iris Clark 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
>