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 ([email protected]) 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)
_______________________________________________ Owncloud mailing list [email protected] https://mail.kde.org/mailman/listinfo/owncloud
