One of the parts mentioned in the original document for setting up the KDE quality team was fixing code reviews.<div><br>Code reviews are probably the most important step to improving quality and so many projects appear to be simply skipping them - or only applying code reviews to "outsiders", and this is a real problem.<br>
<br>I'm currently working my way looking through some piece of code in KDE (I really don't want to name names here) and there's so many constant "mistakes" that I would never have let through code review if I was doing it.<br>
(calling delete on qobjects, leaking objects, unclear APIs, badly named slots, non-virtual destructors, and even a frickin' goto statement in a section of code that created/deleted objects..really horrible stuff) but it's not the author's fault if no-one is reviewing it pointing out what's wrong.<br>
<br>We have mandatory reviews in KDE Telepathy, and it generally works really well. Pretty much everything is reviewed, and I'm not saying our code is perfect, but it's a damn load better than if we didn't have it. Reviews give a chance for code that is perfectly good, to be made better, and we argue a lot about UI/user facing text just as much as the code itself. I wrote about some of the benefits on my blog <a href="http://www.sharpley.org.uk/blog/code-reviews">http://www.sharpley.org.uk/blog/code-reviews</a>.<br>
<br>I don't know why other projects don't do it but I think it's because:<br> - They think it slows development<br> - There's no-one around to review (the only legit reason)<br> - Ego<br> - Laziness/They don't see reviews as useful.<br>
<br>The reason I think it works well for us, is because of the following:<br> - We're really fast at turning round reviews (generally ~30 minutes, up to 2 days max)<br> - We use pastebin + IRC / IM for trivial patches<br>
 - There are lots of us who do reviews. At least 6 of us are happy to say "ship it!" on things, and a few more will participate in the review process.<br> - We all do it, there's no sense of "I'm pro, I get to skip reviews", which means no-one else feels they can skip.<br>
 - If anyone does try to commit something without a review I stab them in the face. <br><br>Conversely I have a patch on plasma that has sat there for a _fortnight_ not going anywhere, and frankly it makes me want to not bother with a review next time. Also I work on another KDE project where I don't bother with reviews, partly because it's in playground and partly because I don't think anyone else would review any changes I did make.<br>
<br><b>So how can we fix it?<br></b><br>It's a mentality change as much as anything else, I don't think going round saying "you don't review, fix it!" will change anything.<br><br>The other problem is that in many cases that I've seen from lurking in IRC channels and alike is that the project leads are setting a bad example. The only fix can come from them changing first, which is a bit of a circular problem because as a project-lead I think it's hardest to find someone willing to review your code.<br>
<br><b>Ideas:<br></b>- create a way for a developer to say "hey, I need more people to review code in my project". (they could email us?)<br><br>- email individual project leaders saying "hey, you don't seem to be doing many code reviews. Why not? Is there anything that can be changed to make that happen". Then get more aggressive.<br>
<br>- break the misnomer that you have to be an expert/know the code to do a code review. Sure it helps, but some review is still better than none. Blogs? Encourage GSOC mentors to make their students review some of the mentor's own code? <br>
<br>I'm not really not sure how else to solve this...<br><br><br>David Edmundson<br><br></div>