[Avocado-devel] [RFC] Avocado Misc Tests repository policy

[Cleber Rosa]

On 08/03/2016 05:07 AM, Amador Pahim wrote:
> Hello,
> 
> We are receiving a big number of Pull Requests in avocado-misc-tests
> repository. If on the one hand it is really great to see the community
> using and contributing to Avocado, on the other hand the Avocado devel
> team should not be that involved in review code of tests, since our
> business is to write Avocado itself.
> 
> This RFC aims to outline the workflow of the Avocado devel team
> regarding the avocado-misc-tests repository.
> 
> Motivation
> ========
> The code review is a time consuming activity and review code of tests
> is not part of the Avocado devel team business. Given the high number
> of Pull Requests on avocado-miscs-tests repository, we need a policy
> to officially state our participation there.
> 
> Proposal
> =======
> We don't believe that we should just not to look to the
> avocado-misc-tests repository. A good number of bugfixes and features
> in Avocado were born as consequence of tests posted there.
> 
> To find a balance, the initial proposal is that the Avocado devel
> team, currently the only maintainers of that repository, will only get
> involved after a third party code review and ACK of a given Pull
> Request. That way, the code author should be also in charge of find
> someone to review its code, being the reviewer from the same company
> or not. We can always invite people to review code there, but it's
> essentially the author's responsibility.
> 
> For the reviewer, it is expected that he/she:
> - Reads the code, commenting with suggestions of improvements: good
> practices, general standards, effectiveness of code, verify comments
> and docstrings.
> - Test the code: run the test and make sure it's working as expected.

I'd suggest, at least as an experiment, to attach the generated job.log
to the review process. GitHub has a "Attach files by dragging & dropping
or selecting them" link.  This can help both process-wise (kind of a
ticking a check box), and for secondary reviewers to debug their
possible failures running the same code.

> - Ping the authors of Pull Requests already reviewed and not updated
> for a long time.
> - When the code is considered ready, comment the Pull Request with an
> 'Looks Good To Me'.
> 
> The 'Looks Good To Me' comment will be the trigger for the maintainers
> to go there and take a final look on the Pull Request and merge it.
> 
> Expected Results
> ==============
> The expected result is to decrease the load of Avocado devel team in
> regards to the code review in that repository.
> Another important expected outcome of this process is to give the
> merge permission for (aka promote to maintainer) those assiduous
> reviewers with good quality of reviews.
> When this RFC is considered ready, we will update our documentation
> and the avocado-misc-test README file to reflect the information.
> 
> Additional Information
> ================
> Any individual willing to make the code review is eligible to do so.
> And the process is simple. Just go there and review the code.
> Given the high volume of code coming from IBM, I had a chat with
> Praveen Pandey, an IBMer and assiduous author of Pull Requests for
> avocado-misc-tests, and he agreed in make reviews in
> avocado-misc-tests.
> 
> 
> Looking forward to read your comments.
> --
> apahim
> 

LGTM.

-- 
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]

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