[Owncloud] [QA-Announcement] Code Reviews to increase quality

[Jan-Christoph Borchardt]

On Tue, Oct 30, 2012 at 12:59 AM, Sam Tuke <samtuke at owncloud.com> wrote:
> On 10/29/2012 11:43 PM, Jan-Christoph Borchardt wrote:
>> TL;DR: Do not directly push to core master, and instead use pull
>> requests.
>
> How much of the procedure is enforced, and how much relies upon convention?

We are all smart people, so I hope we don’t have to enforce much. This
is not about limiting people, but about having a bit of a culture
shift which benefits the quality of ownCloud, away from committing
everything to core which is not reviewed by other people, towards a
pro-active »I better let others review my code first« attitude.


> Who counts as a "reviewer" - should only certain people fulfil this role
> (are there specific reviewers for specific files / apps)?

Kind of, and core committers know / should know who might know about
the code they are changing and can @mention. See 2) and 3) in on
http://owncloud.org/dev/code-reviews-on-github/


> Can people who aren't members of ownCloud on GitHub also act as reviewers?

They are of course welcome to look through the code, test it and
comment, yes. But only a member can pull in the request in the end.


> Do +1s have to take a specific form, or are comments like "looks good"
> sufficient?

On the page it says it needs to be :+1, but I’ll use :rocket: anyway.


> Is it now literally not possible to commit directly to core master?

It still is. I don’t think we developers are stupid and need to be
patronized, so essentially this is more of common sense or an ethereal
code of conduct than a policy to be enforced. It’s bottom-up, not
top-down.