[Owncloud] Pull request testing

Arthur Schiwon blizzz at owncloud.com
Wed Mar 20 10:58:39 UTC 2013


On 03/20/2013 10:57 AM, Klaas Freitag wrote:
> On 19.03.2013 22:28, Frank Karlitschek wrote:
>> I completely agree
>
> I agree partly. Of course all code reviews should be done as accurate as
> possible, and that involves patch applying and testing.

I agree with Klaas

>
> But:
>
>>> A problem is that I often see: »thumbs up, code looks good but I
>>> didn’t test«
>>>
>>> Now I’m sure you’re all code wizards who can parse and render code in
>>> their head … but actually checking out the merge request, open
>>> ownCloud, in an actual browser (or two), and click around a bit if it
>>> really *works* is the crucial part here.
> I don't think that testing of the code changes is always so easy that it
> could be verified by "clicking around a bit". I also don't think that
> code reviews can and should replace a qa step which is an important part
> of bug fixing for which a dev is responsibility.
> Code review here is more like a smoke test where people help to identify
> obvious problems, code errors or design pitfalls.

Also:

Some PRs touch setups or configurations that are not used very often. In 
terms of LDAP there is not a single core developer but me who do run a 
server. While oftenly even two, OpenLDAP and AD, are necessary.

I really benefit from people looking at the code, what works pretty well 
for smaller PRs, but not so much for PRs with big changes.

And it is not only LDAP, but also files_external or things related to 
IIS for instance.

Cheers
Arthur

>
> We should pay attention to not end up with the result that people stand
> away from reviews because we put too much responsibility on the reviewer.
>
> Klaas
>
> _______________________________________________
> Owncloud mailing list
> Owncloud at kde.org
> https://mail.kde.org/mailman/listinfo/owncloud



More information about the Owncloud mailing list