So I can confirm that the requirement of at least one single positive approval is now working (see https://github.com/apache/incubator-pekko-projection/pull/29 as an example). I will commence adding this into other pekko projects in the future.
On Mon, Apr 3, 2023 at 11:40 AM Matthew Benedict de Detrich < [email protected]> wrote: > So after broad consensus on this I have decided to get moving, I created a > PR at Pekko Projection > https://github.com/apache/incubator-pekko-projection/actions/runs/4595089674 > in order to test that there are no side effects and if there are no > problems I will start adding it to the other Pekko modules. > > On Thu, Mar 30, 2023 at 8:39 AM Johannes Rudolph < > [email protected]> wrote: > >> +1 >> >> kerr <[email protected]> schrieb am Mi., 29. März 2023, 18:54: >> >> > +1 >> > 何品 >> > >> > >> > li guobin <[email protected]> 于2023年3月29日周三 17:01写道： >> > >> > > +1 >> > > ________________________________ >> > > 发件人: Greg Methvin <[email protected]> >> > > 发送时间: 2023年3月28日 17:15 >> > > 收件人: [email protected] <[email protected]> >> > > 主题: Re: Enable mandatory single positive review for pull requests in >> > Pekko >> > > projects >> > > >> > > +1 >> > > >> > > On Tue, Mar 28, 2023 at 01:47 Sam Byng <[email protected] >> > >> > > wrote: >> > > >> > > > +1 >> > > > >> > > > > -----Original Message----- >> > > > > From: Matthew Benedict de Detrich <[email protected] >> > .INVALID> >> > > > > Sent: Monday, March 27, 2023 10:08 PM >> > > > > To: [email protected] >> > > > > Subject: [EXTERNAL] Re: Enable mandatory single positive review >> for >> > > pull >> > > > requests in Pekko projects >> > > > > >> > > > > > I think we've been working that way informally, but it makes >> sense >> > to >> > > > > implement a GH check. I'm assuming that the proposal is that the >> > > > approval needs to come from another project committer? >> > > > > >> > > > > While like the most of us I am a fan of patting myself on the >> back I >> > > > would say that counting self reviews would make this check largely >> > > > pointless (so yes, the idea is you need a positive > review from >> > someone >> > > > else). >> > > > > >> > > > > On Mon, Mar 27, 2023 at 9:10 PM Sean Glover <[email protected]> >> > > wrote: >> > > > > >> > > > > +1 >> > > > > >> > > > > I think we've been working that way informally, but it makes >> sense to >> > > > > implement a GH check. I'm assuming that the proposal is that the >> > > > > approval needs to come from another project committer? >> > > > > >> > > > > >> > > > > On Mon, Mar 27, 2023 at 9:09 AM Nicolas Vollmar < >> [email protected]> >> > > > > wrote: >> > > > > >> > > > > > +1 there should be one review required >> > > > > > >> > > > > > On Mon, 27 Mar 2023 at 14:52, Claude Warren, Jr >> > > > > > <[email protected]> wrote: >> > > > > > >> > > > > > > +1 I didn't realize we were not working this way. ;) >> > > > > > > >> > > > > > > On Fri, Mar 24, 2023 at 7:10 PM Matthew Benedict de Detrich >> > > > > > > <[email protected]> wrote: >> > > > > > > >> > > > > > > > There were a couple of cases in the past where PR's were >> merged >> > > > > > > > accidentally without having an actual positive review. How >> do >> > > > > > > > people >> > > > > > feel >> > > > > > > > about preventing PR's from being merged unless they have at >> > > > > > > > least one positive review (with no changes requested)? Note >> > that >> > > > > > > > I am >> > > > > > specifically >> > > > > > > > only asking for a positive review and not other options >> (such >> > as >> > > > > always >> > > > > > > > requiring a branch to be updated with main) since due to the >> > > > > > > > volume >> > > > > of >> > > > > > > pull >> > > > > > > > requests we have now this can become quite counter >> productive. >> > > > > > > > >> > > > > > > > Personally as a minimum bar I find this quite acceptable, we >> > can >> > > > > always >> > > > > > > > increase it/add more checks later down the road as the >> process >> > > > > > progresses >> > > > > > > > (i.e. there is an argument for having Pekko core project >> have 2 >> > > > > > positive >> > > > > > > > reviews rather than one due to how critical it is). >> > > > > > > > >> > > > > > > > -- >> > > > > > > > >> > > > > > > > Matthew de Detrich >> > > > > > > > >> > > > > > > > *Aiven Deutschland GmbH* >> > > > > > > > >> > > > > > > > Immanuelkirchstraße 26, 10405 Berlin >> > > > > > > > >> > > > > > > > Amtsgericht Charlottenburg, HRB 209739 B >> > > > > > > > >> > > > > > > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen >> > > > > > > > >> > > > > > > > *m:* +491603708037 >> > > > > > > > >> > > > > > > > *w:* aiven.io *e:* [email protected] >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > >> > > > Sam Byng >> > > > Software Engineer >> > > > Azure For Operators >> > > > [email protected] >> > > > >> > > > >> > > > >> > > > >> > > > >> > > >> > >> > > > -- > > Matthew de Detrich > > *Aiven Deutschland GmbH* > > Immanuelkirchstraße 26, 10405 Berlin > > Amtsgericht Charlottenburg, HRB 209739 B > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > > *m:* +491603708037 > > *w:* aiven.io *e:* [email protected] > -- Matthew de Detrich *Aiven Deutschland GmbH* Immanuelkirchstraße 26, 10405 Berlin Amtsgericht Charlottenburg, HRB 209739 B Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen *m:* +491603708037 *w:* aiven.io *e:* [email protected]
