+1 Enrico
Il Mer 23 Feb 2022, 07:31 PengHui Li <peng...@apache.org> ha scritto: > +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 > > >
