<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">TL;DR Development in teams is about
communication - add a text comment next to your :+1:<br>
<br>
On 20.03.2013 10:57, Klaas Freitag wrote:<br>
</div>
<blockquote cite="mid:514987F2.6010307@owncloud.com" type="cite">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.
<br>
Code review here is more like a smoke test where people help to
identify obvious problems, code errors or design pitfalls.
<br>
<br>
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.<br>
</blockquote>
<br>
I think I kicked this off with
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
<a href="https://github.com/owncloud/core/pull/2279">https://github.com/owncloud/core/pull/2279</a>.
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*).<br>
<br>
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.<br>
<br>
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.<br>
<br>
So my plea is: Development in teams is about communication - add a
text comment next to your :+1:<br>
<br>
so long<br>
<br>
Jörn<br>
<a href="https://github.com/owncloud/core/pull/2279"></a>
<pre class="moz-signature" cols="72">--
Jörn Friedrich Dreyer (<a class="moz-txt-link-abbreviated" href="mailto:jfd@owncloud.com">jfd@owncloud.com</a>)
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)
</pre>
</body>
</html>