+1 Before I always thought it was Github added this new feature :) Thanks for sharing the great knowledge.
Penghui On Wed, Feb 23, 2022 at 2:24 PM Michael Marshall <mmarsh...@apache.org> wrote: > Hi All, > > In my recent PR to update the `.asf.yaml` to protect release branches, > I set the `dismiss_stale_reviews` to `true` for PRs targeting master > branch [0]. I mistakenly thought this setting would only dismiss PRs > updated by force. Instead, all approvals are dismissed when additional > commits are added to the PR. The GitHub feature is documented here > [1]. > > Since the PR changed the old setting, I want to bring awareness to the > change and determine our preferred behavior before changing the > setting again. > > I think we should return to our old setting [2]. The GitHub PR history > clearly shows when a contributor/committer approved a PR. I feel that > it is up to the "merging" committer to give the final review of the > PR's approval history before merging. Further, when dismiss stale code > reviews is true, GitHub modifies previous approval "history" in the PR > making it look like a reviewer never approved the PR, which I find a > bit confusing. > > Here is a sample PR where approvals were dismissed: [3]. > > Let me know how you think we should proceed. > > Thanks, > Michael > > [0] https://github.com/apache/pulsar/blob/master/.asf.yaml#L76 > [1] > https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/managing-a-branch-protection-rule#creating-a-branch-protection-rule > [2] https://github.com/apache/pulsar/pull/14425 > [3] https://github.com/apache/pulsar/pull/14409 >