Re: [Avocado-devel] Pull request merges for avocado-vt
Dne 18. 01. 19 v 4:15 Xu Han napsal(a): > On Fri, Jan 18, 2019 at 3:03 AM Cleber Rosa wrote: > >> >> >> On 1/15/19 1:20 PM, Plamen Dimitrov wrote: >>> Hi all, >>> >>> I am new to the avocado-vt repo and it seems that merging pull requests >>> there has a different workflow than pull requests in avocado. In >>> particular, it seems that no review is required for a merge - am I >>> correct to assume this? I have 5-6 pull requests waiting and after >>> waiting for some time I just realized that GitHub is giving me green >>> light for each pull request without blocking. Shouldn't a main >>> maintainer also look at the code or is it good to go if it passes all >>> unit tests? >>> >> >> Avocado-VT workflow requires two ACKs, but you're right that the GH UI >> is not preventing merges. >> >> Xu, Lukáš (and others), >> >> Would you like to activate the merge block feature on GH? >> > > That sounds good to me, and honestly it is what I planned to do, so let's > see other maintainers' willingness. > IIRC the reason it was because we have several fairly-active reviewers without write access in Avocado-vt (therefor review is not accounted as "done"), so we left it on our guidelines only. People, especially those with write access, should have read them. The problem is that if we make compulsory to have "2" reviews, it means we always require 2 reviews of people with write access, which is usually not the case. In most cases we get at least 1 "full" ack and 1 "non-core" ack and the PR get's merged by anyone with write access. Requiring 1 ack might theoretically work, but I'm afraid it might lead to: "it was green so I merged it with only 1 ack" situations. So, I'd prefer to keep it as is, we should always inform people that gained write access about this policy (I always do that in the introductory email). I'm not strongly against requiring 1 ack but I'm against requiring 2 acks as that is often not feasible. Regards, Lukáš PS: The green button protection only works when github pages are used. One can always push the changes directly to git (which is what I use, so either way I'm not really affected...) > Thanks, > Xu > > >> >> Thanks, >> - Cleber. >> >>> Best, >>> Plamen >>> >> >> -- >> Cleber Rosa >> [ Sr Software Engineer - Virtualization Team - Red Hat ] >> [ Avocado Test Framework - avocado-framework.github.io ] >> [ 7ABB 96EB 8B46 B94D 5E0F E9BB 657E 8D33 A5F2 09F3 ] >> > signature.asc Description: OpenPGP digital signature
Re: [Avocado-devel] Pull request merges for avocado-vt
On Fri, Jan 18, 2019 at 3:03 AM Cleber Rosa wrote: > > > On 1/15/19 1:20 PM, Plamen Dimitrov wrote: > > Hi all, > > > > I am new to the avocado-vt repo and it seems that merging pull requests > > there has a different workflow than pull requests in avocado. In > > particular, it seems that no review is required for a merge - am I > > correct to assume this? I have 5-6 pull requests waiting and after > > waiting for some time I just realized that GitHub is giving me green > > light for each pull request without blocking. Shouldn't a main > > maintainer also look at the code or is it good to go if it passes all > > unit tests? > > > > Avocado-VT workflow requires two ACKs, but you're right that the GH UI > is not preventing merges. > > Xu, Lukáš (and others), > > Would you like to activate the merge block feature on GH? > That sounds good to me, and honestly it is what I planned to do, so let's see other maintainers' willingness. Thanks, Xu > > Thanks, > - Cleber. > > > Best, > > Plamen > > > > -- > Cleber Rosa > [ Sr Software Engineer - Virtualization Team - Red Hat ] > [ Avocado Test Framework - avocado-framework.github.io ] > [ 7ABB 96EB 8B46 B94D 5E0F E9BB 657E 8D33 A5F2 09F3 ] >
Re: [Avocado-devel] Pull request merges for avocado-vt
On 1/15/19 1:20 PM, Plamen Dimitrov wrote: > Hi all, > > I am new to the avocado-vt repo and it seems that merging pull requests > there has a different workflow than pull requests in avocado. In > particular, it seems that no review is required for a merge - am I > correct to assume this? I have 5-6 pull requests waiting and after > waiting for some time I just realized that GitHub is giving me green > light for each pull request without blocking. Shouldn't a main > maintainer also look at the code or is it good to go if it passes all > unit tests? > Avocado-VT workflow requires two ACKs, but you're right that the GH UI is not preventing merges. Xu, Lukáš (and others), Would you like to activate the merge block feature on GH? Thanks, - Cleber. > Best, > Plamen > -- Cleber Rosa [ Sr Software Engineer - Virtualization Team - Red Hat ] [ Avocado Test Framework - avocado-framework.github.io ] [ 7ABB 96EB 8B46 B94D 5E0F E9BB 657E 8D33 A5F2 09F3 ]
[Avocado-devel] Pull request merges for avocado-vt
Hi all, I am new to the avocado-vt repo and it seems that merging pull requests there has a different workflow than pull requests in avocado. In particular, it seems that no review is required for a merge - am I correct to assume this? I have 5-6 pull requests waiting and after waiting for some time I just realized that GitHub is giving me green light for each pull request without blocking. Shouldn't a main maintainer also look at the code or is it good to go if it passes all unit tests? Best, Plamen signature.asc Description: OpenPGP digital signature
