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 ]
signature.asc
Description: OpenPGP digital signature