1. posterid:900237
  2. Post 2.3.0 PR merging

Re: Post 2.3.0 PR merging

Nashif, Anas
 

Hi,

 

Here some data…

 

What I have just posted on slack…

 

 

So, go do some reviews please 😊

 

 

Thanks

Anas

 

From: <devel@...> on behalf of Carles Cufi <carles.cufi@...>
Date: Tuesday, 7 July 2020 at 06:42
To: Simon Glass <sjg@...>
Cc: "devel@..." <devel@...>, "users@..." <users@...>
Subject: Re: [Zephyr-devel] Post 2.3.0 PR merging

 

Hi Simon,

 

Not to my knowledge, no. But perhaps Ioannis or Anas know if it’s also documented elsewhere. 

 

Regarding the “stamp of approval” that was mentioned earlier in the thread, I think we could adopt a middle ground. If the person with merge rights is reasonably knowledgeable about the area of the change, and understands the implications, their +1 could count even if they are not a maintainer. I feel that in many cases this could unblock PRs without compromising the quality of the code being merged. 

 

Carles 



Am 06.07.2020 um 22:20 schrieb Simon Glass <sjg@...>:

Hi Carles,

 

OK I see. Since you called out the 2-approver change as a cause of the bottleneck, I'm assuming there is data available on this.

 

I was looking around for discussion about the decision. The closest I found was here. Is there something else?

 

Regards,

SImon

 

 

On Mon, 6 Jul 2020 at 12:23, Cufi, Carles <Carles.Cufi@...> wrote:

Hi Simon,

 

A decision like that would need to go through the TSC, and in order to be able to vote we’d need to have clear stats on how many PRs are actually blocked by this policy. I was thinking about this recently, and given that those in Zephyr with merge rights can in fact add their +1 and they typically rely on a maintainer’s review to decide whether to merge a PR or not, this might not have as big as an impact as I originally thought.

 

That said, I believe we need a new GitHub filter to help us. Right now, we have one that lists PRs that are approved, passing CI and ready to merge, but what we need is one that shows all PRs that are partially approved (i.e. a single +1) and passing CI. Then those of us with merge rights could go over those regularly (like we do for the former) and, provided there’s a +1 from a maintainer, we could take one additional look at the PR, approve it and merge it.

 

I’d very much like to hear the opinion of others with merge rights in this thread.

 

Regards,

 

Carles

 

From: Simon Glass <sjg@...>
Sent: 06 July 2020 19:46
To: Cufi, Carles <Carles.Cufi@...>
Cc: devel@...; users@...
Subject: Re: [Zephyr-devel] Post 2.3.0 PR merging

 

Hi Carles,

 

On Fri, 12 Jun 2020 at 03:19, Carles Cufi <carles.cufi@...> wrote:

Hi all,

We currently have 170+ Pull Requests that pass CI but require one or more approvals in order to be merged, in part due to the fact that we have increased the minimum number of approvals required from 1 to 2.

 

Perhaps given the situation this should be reversed?

 

Regards,

Simon 

 


Help reviewing those PRs is greatly appreciated:

https://github.com/zephyrproject-rtos/zephyr/pulls?q=is%3Apr+is%3Aopen+base%3Amaster+review%3Arequired+status%3Asuccess+-label%3ADNM+draft%3Afalse

Thanks,

Carles
Join devel@lists.zephyrproject.org to automatically receive all group messages.