+1 (binding)
> On 8.05.2017 г., at 19:03, Andrea Turli <[email protected]>
> wrote:
>
> +1 (binding)
>
> On 8 May 2017 at 17:45, Aled Sage <[email protected]> wrote:
>
>> +1 (binding)
>>
>>
>>
>> On 08/05/2017 11:55, Richard Downer wrote:
>>
>>> There have been recent discussions about how the committers assess PRs for
>>> merging. The discussion is summarised below and the original thread
>>> available at [1].
>>>
>>> The consensus of the discussion is to adopt new standards for committers
>>> reviewing PRs, as follows:
>>>
>>> ------
>>>
>>> If a PR has not been reviewed within a certain amount of time - suggested
>>> to be 7 days, or less for smaller PRs - nor has a committer indicated that
>>> they are doing a detailed review, then the PR shall be considered for a
>>> less detailed review, an "eyeball test".
>>>
>>> Under an eyeball test, a reviewer will consider if the PR is:
>>> * clearly helpful & not obviously wrong
>>> * low-risk / doesn't break compatibility
>>> * good test coverage (and passing)
>>> * likely to be maintained
>>>
>>> If it passes the above criteria, then the reviewer will add a comment to
>>> the PR, and ask if further review is appropriate, possibly tagging
>>> specific
>>> committers who may be interested. Then if there is no objection within 72
>>> hours, passive consensus should be assumed, and the PR merged.
>>>
>>> If the PR does not pass the above criteria, the reviewer should say what
>>> they have doubts about, suggest what the contributor could do to help,
>>> and/or appeal to other committers more familiar with an area. If
>>> appropriate, move from GitHub onto the mailing list. (The aim here is to
>>> get a discussion going and not give the contributor the impression their
>>> PR
>>> is being ignored.)
>>>
>>> ------
>>>
>>> This vote is to decide if we wish to adopt these standards for all PR
>>> reviews going forward, and to document these standards in our website.
>>>
>>> This vote will be open for a minimum of 72 hours.
>>>
>>> [ ] +1 - adopt this standard
>>> [ ] 0 - no opinion
>>> [ ] -1 - do not adopt this standard, because:
>>>
>>> ------
>>>
>>> Background:
>>>
>>> This is related to the recent thread at [1].
>>>
>>> Traditionally, this project has had a high bar for reviewing contributions
>>> prior to merging. This dates back to the project's inception, before it
>>> was
>>> part of Apache. Reviewers would be expected to inspect the code and
>>> personally test it before allowing it to be merged.
>>>
>>> There has been concern expressed that this is holding back Brooklyn
>>> development. Reviewing a PR can be time-consuming; often a detailed review
>>> requires expert knowledge in a particular area of the code which only some
>>> committers possess. The result is that PRs, especially larger ones or ones
>>> in core areas of the project, do not receive timely review, and in some
>>> cases languish far too long. This is bad for the project as it holds back
>>> our velocity, and frustrates contributors who see their changes stuck in
>>> the system for extended lengths of time.
>>>
>>> Since we joined the ASF, we have had feedback from others with experience
>>> in Apache that we are too conservative with our code review requirements.
>>> We also recognise the value in automated testing to catch regressions
>>> (although these constantly need work, of course), and in our Git source
>>> control to enable us to revert changes that turn out to be particularly
>>> problematic. We can relax our strict reviewing requirements, which will
>>> increase our velocity, and show our contributors that their work is
>>> receiving attention and getting merged. Should a merge prove to be
>>> problematic, their is still opportunity to do a bug fix (and get it merged
>>> under the same fast process, too), and ultimately the chance to "revert"
>>> the merge if necessary.
>>>
>>> So we believe that the quality of the finished product will not be
>>> adversely affected by these changes.
>>>
>>>
>>> [1]
>>> https://lists.apache.org/thread.html/4398448fd548495a5159016
>>> a97afa12dd787ab34786b3bbc0881d5b4@%3Cdev.brooklyn.apache.org%3E
>>>
>>> Thanks
>>> Richard.
>>>
>>>
>>
