Seems like the consensus is to make this change. I'll follow up with infra.
-Dan On Wed, Oct 30, 2019 at 11:06 AM Jason Huynh <jhu...@pivotal.io> wrote: > +1, thanks Dan! > > On Wed, Oct 30, 2019 at 10:07 AM Aaron Lindsey <alind...@pivotal.io> > wrote: > > > +1 > > > > - Aaron > > > > > > On Wed, Oct 30, 2019 at 8:02 AM Ju@N <jujora...@gmail.com> wrote: > > > > > Perfect Naba, thanks for answering this. > > > My vote is +1 then. > > > > > > On Wed, Oct 30, 2019 at 2:37 PM Nabarun Nag <n...@apache.org> wrote: > > > > > > > The check box Dan is mentioning will just not invalidate any approved > > > > review if the code is changed. > > > > If a change is requested, the button will remain disabled. > > > > > > > > Regards > > > > Naba > > > > > > > > > > > > On Wed, Oct 30, 2019 at 6:27 AM Joris Melchior <jmelch...@pivotal.io > > > > > > wrote: > > > > > > > > > +1 > > > > > > > > > > On Wed, Oct 30, 2019 at 5:27 AM Ju@N <jujora...@gmail.com> wrote: > > > > > > > > > > > Question: this only applies for *approvals*, not for *refusals*, > > > > right?; > > > > > I > > > > > > mean, the *merge pull request* button will remain blocked if > there > > > were > > > > > > some changes requested by reviewers and the author of the PR adds > > new > > > > > > commits (either addressing those requested changes or not)?. > > > > > > If the answer to the above is "yes", then +1. > > > > > > > > > > > > On Wed, Oct 30, 2019 at 1:44 AM Nabarun Nag <n...@apache.org> > > wrote: > > > > > > > > > > > > > +1 > > > > > > > > > > > > > > On Tue, Oct 29, 2019 at 6:21 PM Darrel Schneider < > > > > > dschnei...@pivotal.io> > > > > > > > wrote: > > > > > > > > > > > > > > > +1 > > > > > > > > > > > > > > > > On Tue, Oct 29, 2019 at 6:08 PM Owen Nichols < > > > onich...@pivotal.io> > > > > > > > wrote: > > > > > > > > > > > > > > > > > +1 …this has already bitten me a few times > > > > > > > > > > > > > > > > > > > On Oct 29, 2019, at 6:01 PM, Dan Smith < > dsm...@pivotal.io> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > It seems we've configured our branch protection rules > such > > > that > > > > > > > > pushing a > > > > > > > > > > change to a PR that has been approved invalidates the > > > previous > > > > > > > > approval. > > > > > > > > > > > > > > > > > > > > I think we should turn this off - it looks like it's an > > > > optional > > > > > > > > feature. > > > > > > > > > > We should trust people to rerequest reviews if needed. > > Right > > > > now > > > > > > this > > > > > > > > is > > > > > > > > > > adding busywork for people to reapprove minor changes > > (Fixing > > > > > merge > > > > > > > > > > conflicts, spotless, etc.) > > > > > > > > > > > > > > > > > > > > If you all agree I'll ask infra to uncheck "Dismiss stale > > > pull > > > > > > > request > > > > > > > > > > approvals when new commits are pushed." in our branch > > > > protection > > > > > > > rules. > > > > > > > > > > > > > > > > > > > > -Dan > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Ju@N > > > > > > > > > > > > > > > > > > > > > -- > > > > > *Joris Melchior * > > > > > CF Engineering > > > > > Pivotal Toronto > > > > > 416 877 5427 > > > > > > > > > > “Programs must be written for people to read, and only incidentally > > for > > > > > machines to execute.” – *Hal Abelson* > > > > > <https://en.wikipedia.org/wiki/Hal_Abelson> > > > > > > > > > > > > > > > > > > -- > > > Ju@N > > > > > >
