You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pekko.apache.org by li guobin <dr...@outlook.com> on 2023/03/29 09:01:15 UTC
回复: Enable mandatory single positive review for pull requests in Pekko projects
+1
________________________________
发件人: Greg Methvin <gr...@apache.org>
发送时间: 2023年3月28日 17:15
收件人: dev@pekko.apache.org <de...@pekko.apache.org>
主题: Re: Enable mandatory single positive review for pull requests in Pekko projects
+1
On Tue, Mar 28, 2023 at 01:47 Sam Byng <sa...@microsoft.com.invalid>
wrote:
> +1
>
> > -----Original Message-----
> > From: Matthew Benedict de Detrich <ma...@aiven.io.INVALID>
> > Sent: Monday, March 27, 2023 10:08 PM
> > To: dev@pekko.apache.org
> > 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 <se...@seanglover.com> 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 <nv...@gmail.com>
> > wrote:
> >
> > > +1 there should be one review required
> > >
> > > On Mon, 27 Mar 2023 at 14:52, Claude Warren, Jr
> > > <cl...@aiven.io.invalid> 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
> > > > <ma...@aiven.io.invalid> 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:* matthew.dedetrich@aiven.io
> > > > >
> > > >
> > >
> >
>
>
> Sam Byng
> Software Engineer
> Azure For Operators
> sambyng@microsoft.com
>
>
>
>
>
Re: Enable mandatory single positive review for pull requests in Pekko projects
Posted by Matthew Benedict de Detrich <ma...@aiven.io.INVALID>.
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 <
matthew.dedetrich@aiven.io> 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 <
> johannes.rudolph@gmail.com> wrote:
>
>> +1
>>
>> kerr <he...@gmail.com> schrieb am Mi., 29. März 2023, 18:54:
>>
>> > +1
>> > 何品
>> >
>> >
>> > li guobin <dr...@outlook.com> 于2023年3月29日周三 17:01写道：
>> >
>> > > +1
>> > > ________________________________
>> > > 发件人: Greg Methvin <gr...@apache.org>
>> > > 发送时间: 2023年3月28日 17:15
>> > > 收件人: dev@pekko.apache.org <de...@pekko.apache.org>
>> > > 主题: Re: Enable mandatory single positive review for pull requests in
>> > Pekko
>> > > projects
>> > >
>> > > +1
>> > >
>> > > On Tue, Mar 28, 2023 at 01:47 Sam Byng <sambyng@microsoft.com.invalid
>> >
>> > > wrote:
>> > >
>> > > > +1
>> > > >
>> > > > > -----Original Message-----
>> > > > > From: Matthew Benedict de Detrich <matthew.dedetrich@aiven.io
>> > .INVALID>
>> > > > > Sent: Monday, March 27, 2023 10:08 PM
>> > > > > To: dev@pekko.apache.org
>> > > > > 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 <se...@seanglover.com>
>> > > 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 <
>> nvollmar@gmail.com>
>> > > > > wrote:
>> > > > >
>> > > > > > +1 there should be one review required
>> > > > > >
>> > > > > > On Mon, 27 Mar 2023 at 14:52, Claude Warren, Jr
>> > > > > > <cl...@aiven.io.invalid> 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
>> > > > > > > <ma...@aiven.io.invalid> 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:* matthew.dedetrich@aiven.io
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > > >
>> > > > Sam Byng
>> > > > Software Engineer
>> > > > Azure For Operators
>> > > > sambyng@microsoft.com
>> > > >
>> > > >
>> > > >
>> > > >
>> > > >
>> > >
>> >
>>
>
>
> --
>
> 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:* matthew.dedetrich@aiven.io
>
--
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:* matthew.dedetrich@aiven.io
Re: Enable mandatory single positive review for pull requests in Pekko projects
Posted by Matthew Benedict de Detrich <ma...@aiven.io.INVALID>.
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 <jo...@gmail.com>
wrote:
> +1
>
> kerr <he...@gmail.com> schrieb am Mi., 29. März 2023, 18:54:
>
> > +1
> > 何品
> >
> >
> > li guobin <dr...@outlook.com> 于2023年3月29日周三 17:01写道：
> >
> > > +1
> > > ________________________________
> > > 发件人: Greg Methvin <gr...@apache.org>
> > > 发送时间: 2023年3月28日 17:15
> > > 收件人: dev@pekko.apache.org <de...@pekko.apache.org>
> > > 主题: Re: Enable mandatory single positive review for pull requests in
> > Pekko
> > > projects
> > >
> > > +1
> > >
> > > On Tue, Mar 28, 2023 at 01:47 Sam Byng <sa...@microsoft.com.invalid>
> > > wrote:
> > >
> > > > +1
> > > >
> > > > > -----Original Message-----
> > > > > From: Matthew Benedict de Detrich <matthew.dedetrich@aiven.io
> > .INVALID>
> > > > > Sent: Monday, March 27, 2023 10:08 PM
> > > > > To: dev@pekko.apache.org
> > > > > 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 <se...@seanglover.com>
> > > 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 <
> nvollmar@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > +1 there should be one review required
> > > > > >
> > > > > > On Mon, 27 Mar 2023 at 14:52, Claude Warren, Jr
> > > > > > <cl...@aiven.io.invalid> 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
> > > > > > > <ma...@aiven.io.invalid> 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:* matthew.dedetrich@aiven.io
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > > Sam Byng
> > > > Software Engineer
> > > > Azure For Operators
> > > > sambyng@microsoft.com
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> >
>
--
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:* matthew.dedetrich@aiven.io
Re: Enable mandatory single positive review for pull requests in Pekko projects
Posted by Johannes Rudolph <jo...@gmail.com>.
+1
kerr <he...@gmail.com> schrieb am Mi., 29. März 2023, 18:54:
> +1
> 何品
>
>
> li guobin <dr...@outlook.com> 于2023年3月29日周三 17:01写道：
>
> > +1
> > ________________________________
> > 发件人: Greg Methvin <gr...@apache.org>
> > 发送时间: 2023年3月28日 17:15
> > 收件人: dev@pekko.apache.org <de...@pekko.apache.org>
> > 主题: Re: Enable mandatory single positive review for pull requests in
> Pekko
> > projects
> >
> > +1
> >
> > On Tue, Mar 28, 2023 at 01:47 Sam Byng <sa...@microsoft.com.invalid>
> > wrote:
> >
> > > +1
> > >
> > > > -----Original Message-----
> > > > From: Matthew Benedict de Detrich <matthew.dedetrich@aiven.io
> .INVALID>
> > > > Sent: Monday, March 27, 2023 10:08 PM
> > > > To: dev@pekko.apache.org
> > > > 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 <se...@seanglover.com>
> > 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 <nv...@gmail.com>
> > > > wrote:
> > > >
> > > > > +1 there should be one review required
> > > > >
> > > > > On Mon, 27 Mar 2023 at 14:52, Claude Warren, Jr
> > > > > <cl...@aiven.io.invalid> 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
> > > > > > <ma...@aiven.io.invalid> 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:* matthew.dedetrich@aiven.io
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> > > Sam Byng
> > > Software Engineer
> > > Azure For Operators
> > > sambyng@microsoft.com
> > >
> > >
> > >
> > >
> > >
> >
>
Re: Enable mandatory single positive review for pull requests in Pekko projects
Posted by kerr <he...@gmail.com>.
+1
何品
li guobin <dr...@outlook.com> 于2023年3月29日周三 17:01写道：
> +1
> ________________________________
> 发件人: Greg Methvin <gr...@apache.org>
> 发送时间: 2023年3月28日 17:15
> 收件人: dev@pekko.apache.org <de...@pekko.apache.org>
> 主题: Re: Enable mandatory single positive review for pull requests in Pekko
> projects
>
> +1
>
> On Tue, Mar 28, 2023 at 01:47 Sam Byng <sa...@microsoft.com.invalid>
> wrote:
>
> > +1
> >
> > > -----Original Message-----
> > > From: Matthew Benedict de Detrich <ma...@aiven.io.INVALID>
> > > Sent: Monday, March 27, 2023 10:08 PM
> > > To: dev@pekko.apache.org
> > > 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 <se...@seanglover.com>
> 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 <nv...@gmail.com>
> > > wrote:
> > >
> > > > +1 there should be one review required
> > > >
> > > > On Mon, 27 Mar 2023 at 14:52, Claude Warren, Jr
> > > > <cl...@aiven.io.invalid> 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
> > > > > <ma...@aiven.io.invalid> 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:* matthew.dedetrich@aiven.io
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> > Sam Byng
> > Software Engineer
> > Azure For Operators
> > sambyng@microsoft.com
> >
> >
> >
> >
> >
>