[Owncloud] Pull request testing

Jörn Friedrich Dreyer jfd at owncloud.com
Wed Mar 20 10:33:14 UTC 2013


TL;DR Development in teams is about communication - add a text comment
next to your :+1:

On 20.03.2013 10:57, Klaas Freitag wrote:
> 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.
>
> 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.

I think I kicked this off with
https://github.com/owncloud/core/pull/2279. Most of the developers will
give their :+1: if they feel the PR is okeyish enough, or if they just
want something in, for whatever reason. Personally, I am happy to give
my :+1: without checking out the branch when a PR is small enough and I
am familiar with the code to estimate if it will improve the situation
or make it worse - yes, this is a gut feeling and I may well be
overconfident sometimes. Nevertheless, with increasing complexity of the
PR, such as #2279 - which refactors the complete upload js part, I get
reluctant to just merge it ... because my gut tells me: "I will be in
sooooo much trouble if this breaks". I think this is just human and ok.
And I think this is another great vote for automated tests ;) because If
my PR survives Jenkins testing I can rest easy (if the testsuite is big
enough, that is *hint *hint*).

Development in teams is all about communication and I think github makes
that very easy, so I requested a more thorough review in the PR for
#2279 when I felt it needed more testing. We don't need to burden
reviewers with strict rules. Asking for a "good and hard testing" as
Frank would say is a good way to signal people that the PR might break
stuff.

Personally, I interpret ":+1:" as yes I want this. And if people add
something like "works in Opera", "tested in IE" or comment on specific
LoC I have a lot more information to work with.

So my plea is: Development in teams is about communication - add a text
comment next to your :+1:

so long

Jörn
<https://github.com/owncloud/core/pull/2279>

-- 
Jörn Friedrich Dreyer (jfd at owncloud.com)
Software Developer
ownCloud GmbH

Your Data, Your Cloud, Your Way!

ownCloud GmbH, GF: Markus Rex, Holger Dyroff
Schloßäckerstrasse 26a, 90443 Nürnberg, HRB 28050 (AG Nürnberg)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/owncloud/attachments/20130320/19839282/attachment.html>


More information about the Owncloud mailing list