[Avocado-devel] Review and merge rules

[Plamen Dimitrov]

Hi Cleber,

On 12/4/19 7:18 PM, Cleber Rosa wrote:
> On Wed, Dec 04, 2019 at 03:05:55PM +0200, Plamen Dimitrov wrote:
>> Hi all,
>>
>> I am fairly new to the avocado community (about a year or so) and I am not
>> sure so far I was clear on the reviewing and merging authorization that is
>> employed in all avocado repositories. I was left with the apparently false
>> impression that new members of the community can only comment on newer PR-s
>> and nothing else and merging a PR would require two reviews from authorized
>> members. However, recently I noticed that we can also review open pull
>> requests which I didn't know before. Is this true or just wrongly enabled
>> github switch? If so are there formal rules who can review/merge/comment
>> anywhere to be found in the official avocado documentation or is it a standard
>> workflow with the only rules that:
> Hi Plamen,
> 
> In fact the general policies used vary between the various Avocado
> repositories.  For instance, on Avocado-VT, the "two reviews" rule
> apply, while in Avocado we don't have such a strict requirement on
> the number of reviews.

I see, it makes sense now and definitely clears things up.

>> 1) two reviews are needed for merge for everyone
> This is general rules for Avocado:
> 
>   https://avocado-framework.readthedocs.io/en/73.0/guides/contributor/chapters/how.html#code-review
> 
> It may be that GitHub prevents non-contributors from submitting formal
> "Approved" reviews.  I myself consider a comment with a "Reviewed,
> LGTM to me" just as good.

I had a bit of experience today from this and it seems GitHub allows
official reviews/approvals from contributors for sure.

> For Avocado-VT I'm mentally aware of the "two reviews" rule, but I
> can't find that in the docs.
> 
>> 2) everyone can review new pull requests
> With regards to reviews, anyone *should* be able to review PRs.  I'd
> consider the opposite to be a badly configured GitHub switch.  I don't
> think we have that explicitly stated anywhere.  I have to say I took
> that for granted.
> 
> Most importantly IMO is: do you think there's room for improvements
> here?  If so, what/how?
> 

If you are referring to the documentation state of this workflow, then I
noticed that indeed the avocado situation on the matter has been well
explained on the above link. Perhaps a single extra sentence explicitly
encouraging reviews from contributors would make both points above clear.
In the case of Avocado VT, I understand that differences in review process
and overall contribution workflow differs mostly due to historical reasons.
I am not sure if it is even possible to standardize these more across the
avocado projects.

Thanks a lot for asking this,
Plamen

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20191204/b8a8d810/attachment.sig>