Closing the loop, we merged the PR to set `dismiss_stale_reviews` to `false` two days ago.
Thanks, Michael On Wed, Feb 23, 2022 at 2:52 AM Li Li <urfreesp...@gmail.com> wrote: > > +1 > > > On Feb 23, 2022, at 4:23 PM, Guangning E <eguangn...@gmail.com> wrote: > > > > +1 > > > > > > Thanks, > > Guangning > > > > Enrico Olivelli <eolive...@gmail.com> 于2022年2月23日周三 16:01写道： > > > >> +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 > >>>> > >>> > >> >
