| |||||||||||||
Where have the reviewers gone?Where have the reviewers gone?Posted May 24, 2007 8:52 UTC (Thu) by nix (subscriber, #2304)In reply to: Where have the reviewers gone? by neilbrown Parent article: Where have the reviewers gone?
People have commented in the past that if they turn out near-perfect code
(Log in to post comments)
Where have the reviewers gone? Posted May 24, 2007 9:33 UTC (Thu) by MathFox (guest, #6104) [Link] It very much depends on the review-instructions. I have done code reviews on various (proprietary) projects and on one project we had an pretty simple and effective review guideline:Would you accept the responsibility to maintain (fix bugs) in the code?The goal wan not to get 100% bug free code, but to get clean and fixable code. (If we saw flaws, we pointed them out; most of them were then quickly fixed.) I can imagine that PostgreSQL places the bar a little higher and looks for data integrity and security issues too. Anyway, it is reasonable to expect a peer review to take 5-10% of the coding time; when you need more, the code has become too complex and is ripe for a (re)design.
Where have the reviewers gone? Posted May 24, 2007 13:56 UTC (Thu) by nix (subscriber, #2304) [Link] Oh dear, that wouldn't work very well with me, or with anyone else who has a chronic inability to say no :/
Where have the reviewers gone? Posted May 24, 2007 19:28 UTC (Thu) by smoogen (subscriber, #97) [Link] Well when we dealt with it.. we stuck the person who said yes until we either realized that their input was useless or they really meant that they would support the code. The opposite also showed up. If a person constantly said no because they didnt want to do the work later of supporting it.. they found that their votes were less valued (or asked for).
Where have the reviewers gone? Posted May 24, 2007 23:46 UTC (Thu) by giraffedata (subscriber, #1954) [Link] I think that's a very vague standard. I myself would pass just about any code under that standard, because if I were to have responsibility for supporting it, I could just rewrite it all to meet my requirements. I have in fact accepted responsibility for supporting code (often for money) that I thought was poorly written and had various apparent bugs.And on the other hand, I would hate to have someone review my own code under that standard, because a reviewer can interpret it as meaning if my code isn't written the way he would do it, I can't ship it. I.e. I must function as some stranger's lackey. A clear definition of the meaning of a review is essential to eliminating the arguments, frustrations, and hurt feelings that turn people off of both sides of reviewing (don't want to review; don't want to submit code for review). And yet, I almost never see it. And, strangely, most people don't seem to notice it missing. There is quite a variety of activity that can be a review. One kind of a review is simply advice. You're pointing out to the coder things he may have missed. You've satisfied your obligation if you've taken a reasonable look at the code and communicated every issue that you think he may have overlooked. The code is the responsibility of the coder. On the other end of the scale is acceptance of the code. The reviewer owns it and has full responsibility for it. He can reject it for any reason without even stating a reason -- correctness, readability, indentation, uselessness. He can also accept it without reading it. My point is that if the coder and reviewer are working off different definitions, the review is likely to be unpleasant for both and I could see how people wouldn't volunteer to review code.
|
|||||||||||||