Re: [Zephyr-users] [Zephyr-devel] Post 2.3.0 PR merging
Nashif, Anas
Agreed. Those who can merge, should take 2 approvals and all other checks as a sign that it is ready to merge, no need for the approval of the person who merges, although a 3rd approval might give the PR a strong “Go”. This is actually what we agreed on 😊
Anas
From:
<users@...> on behalf of Lawrence King <lawrence.king@...>
Hi Charles:
This just my personal opinion on the subject. Two approvals is the right thing to do and what the TSC approved earlier this year.
BUT, if the second +1 is just a rubber stamp approval of the maintainer’s +1 then there is no point having two approvals.
The second +1 should come from someone who is: knowledgeable in the area, has studied the changes, and approves.
Lawrence King Principal Developer +1(416)627-7302
From: devel@... <devel@...>
On Behalf Of Carles Cufi
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@...>
Hi Carles,
On Fri, 12 Jun 2020 at 03:19, Carles Cufi <carles.cufi@...> wrote:
Perhaps given the situation this should be reversed?
Regards, Simon
|
|