Slashdot is powered by your submissions, so send in your scoop

 



Forgot your password?
typodupeerror
×
Programming

Are You Too Good For Code Reviews? 495

theodp writes "Why do some programmers,' asks Chris Hemedinger, 'place little value on code reviews?' This apparently includes even Programming Greats like Ken 'C' Thompson, who quipped, 'we were all pretty good coders' when asked about the importance of code reviews in his work. Hemedinger, on the other hand, subscribes to the school of thought that peer code reviews are Things Everyone Should Do. Not only do reviews keep you on your toes, Hemedinger says, they also 'improve quality, ensure continuity, and keep it fresh. Who can argue against that?'"
This discussion has been archived. No new comments can be posted.

Are You Too Good For Code Reviews?

Comments Filter:
  • by Joe_Dragon ( 2206452 ) on Friday July 08, 2011 @09:52AM (#36694690)

    And some of that needs to testers who are NOT coders or people who are not mainly doing programming.

    • I would also say we need more testers who 'are' coders, just not 'the' coders of the software being tested.

      With automated testing coming along in it's depth and flexibility, implementing the tests themselves is becoming it's own version of programming.

      One of my biggest gripes is testers who don't know SQL. How do you set up your test cases without preloading the database? If you're manually entering data through the app you're testing you still need to verify that data got input correctly and that i
      • automated testing can let stuff fail but still pass the auto tests and it still misses the GUI / user experience testing.

        • by pixelpusher220 ( 529617 ) on Friday July 08, 2011 @10:30AM (#36695232)
          automated testing is primarily for regression testing anyway. GUI / user experience 'testing' is usually after that point, since 'GUI' is just the interface and not the back end logic. Working but looking bad is inherently preferable to looking good but not working....
          • by Pieroxy ( 222434 )

            When your input field is overlapped by your nice little logo, there's no way to enter anything in it (for the average Joe). You page may "work" but if users can't figure out how, it it as good as a broken database.

          • by mcvos ( 645701 )

            Functionality tests can also be important for regression testing. I remember a ruby site I once worked on where, due to some change, registration of a new user didn't work anymore. All the individual classes worked fine, but something wasn't redirected correctly, or not properly stored in the database or something like that. Automated functionality tests allowed us to keep an eye on that.

      • One of my biggest gripes is testers who don't know SQL. How do you set up your test cases without preloading the database? If you're manually entering data through the app you're testing you still need to verify that data got input correctly and that involves looking under the hood in the actual DB.

        And yet we wonder why there is a rash of SQL injection attacks on public websites and servers by teenagers who are pissed at the world.

    • by Z00L00K ( 682162 )

      But testing is not code reviewing.

      However there are tools that can help a lot when doing code review. But you also have to agree on certain policies and also agree on what shall be ignored since some review remarks dropped by those tools are just foolish or counter-productive.

      As for testing - that's a later stage in the process of development.

      • As for testing - that's a later stage in the process of development.

        Apparently by teenagers and young adults angry at the world who like nothing more than to 'test' websites with a nice little Iranian program, complete with a GUI and tutorial. They are all to happy to share their results with as many people as possible... all for free. Normally you have to pay for that:P

      • Yeah, goodness. Maybe if all the lazy IT support and testing staff would quit trolling /. and get back to their job we could get back to the original topic here. Code Reviews by coders who know how to write code reviewing each other's code. Not UI crap, or testing in general.
      • by perpenso ( 1613749 ) on Friday July 08, 2011 @11:04AM (#36695812)

        As for testing - that's a later stage in the process of development.

        Testing and code reviews should occur at the earliest possible moment and be integrated throughout development. Bugs cost less when they are found earlier.

      • by arth1 ( 260657 )

        The problem with code review is that it's expensive. For someone else to go through all the code to the point that they understand all of it, they have to invest at least as much time as the coder.
        They don't, so they don't, so the code reviews become skimming for obvious flaws, which is well and good, but it isn't a code review.
        To do it properly, your code becomes twice as expensive. That's cheap, but try explaining that to a PHB.

    • by seyyah ( 986027 )

      And some of that needs to testers who are NOT coders or people who are not mainly doing programming.

      Dude, you need a comment reviewer.

  • Pure Arrogance (Score:2, Informative)

    by Anonymous Coward

    Anyone who is anti-code review is an arrogant snot. No one is perfect, and we all have the capacity to overlook things.

    • Code review is purposefully a politically loaded process which enables management to divide and conquer and keep wages down.

      Such processes can get hung up on stupid details when coder A's readability is better than coder B and C's, but unfortunately for A, is also not "standard" and s/he gets slammed for it and told to conform even if it makes for slightly less readable code just because it satisfies some corporate "this is how we do it and you don't question it".

      I think code reviews would be much more valu

      • by Surt ( 22457 )

        Code review can be that, but doesn't have to be. For example, at my place of work, code review results aren't documented, and can't be brought up during personnel review. The purpose of code review is quality (for us).

  • I agree 100% (Score:5, Insightful)

    by jlechem ( 613317 ) on Friday July 08, 2011 @09:54AM (#36694710) Homepage Journal
    I have worked in places with and without code reviews. Yes code reviews can be painful especially if you work with some douchebags. But overall getting new eyeballs looking at your code often brings up things you hadn't thought about, etc. It doesn't make you or the other people bad, it's just how it is. Drop the ego and take some constructive feedback.
    • Re:I agree 100% (Score:5, Insightful)

      by flooey ( 695860 ) on Friday July 08, 2011 @09:59AM (#36694770)

      Yes code reviews can be painful especially if you work with some douchebags.

      I don't think the problem there is the code reviews.

      • by jlechem ( 613317 )
        Exactly the code review itself isn't bad, it's the personal interaction between coworkers that has always been the issue from what I've seen.
    • Re:I agree 100% (Score:4, Interesting)

      by Tanktalus ( 794810 ) on Friday July 08, 2011 @11:08AM (#36695868) Journal

      I have a huge ego. I freely admit it. I work from home because I can't fit my ego in the office doors.

      On a serious note, I prefer code reviews. From the ego-centric side, it has two major benefits: it allows my ego to show off, second it helps the less-experienced/newer developers to learn new techniques, or just learn the code, often in areas they haven't spent a lot of time looking at. That means less time helping them later. It also happens to have the side benefit that their questions can make me think about the problem harder, and not-infrequently uncover typos, thinkos, under-developed (or over-developed) features, or plain bugs. Even for us egomaniacs.

    • Re:I agree 100% (Score:4, Interesting)

      by TheRaven64 ( 641858 ) on Friday July 08, 2011 @11:18AM (#36696046) Journal

      I've found that code reviews are most productive if you pair experienced and inexperienced coders. An experienced coder is probably writing good functional code, but may not be writing code that's easy to understand. Having code reviewed by someone less experienced helps catch things where someone new to the codebase would struggle to understand the code. This should prompt additional comments or (ideally) refactoring into a clearer form. The inexperienced developer also benefits from being forced to read and understand someone else's (hopefully good) code. In the other direction, the inexperienced developer gets helpful feedback on their design and techniques.

      Code reviews done by someone with a similar skill level tend to be a waste of time. They'd typically implement things in more or less the same way, so they either skip over things and don't give much feedback or they're really pedantic but not very helpful.

  • by blahplusplus ( 757119 ) on Friday July 08, 2011 @09:55AM (#36694724)

    ... code is good enough.

    There are times when code reviews make sense and times where they don't, having the wisdom to know the difference is key.

    • by Z00L00K ( 682162 )

      And that's when it builds with no warnings whatsoever using the most paranoid settings and with a lint analysis that's silent. Then it's good enough.

      • Ohh, and it does what the customer expected...or was that part optional?

      • Warnings and lint are a start, but they say nothing about whether or not things actually work.

        When the required functionality is specified, and a test procedure that actually tests the required functionality (to the degree appropriate for each function's level of concern) passes successfully, then I'll say that the code itself doesn't need further review.

        When new specs come along, or a new programmer is added to the project, some form of code review is called for.

    • by MobyDisk ( 75490 )

      having the wisdom to know the difference is key.

      Wisdom would dictate that the author is incapable of judging when it is good enough. Ergo, even if it is already good enough, the code review is necessary in order to make that determination. Fortunately, good code is relatively easy to review.

  • I love code reviews; I usually like doing them and I really want my code reviewed too. But it can be like pulling teeth to get co-workers to do them in a timely manner. So yeah, I commit unreviewed code unless it feels risky.

    • I love code reviews; I usually like doing them and I really want my code reviewed too.

      I agree, it's one of the better learning experiences short of 'pair programming' (which I can't stand).

      The single biggest problem I have is that time is never budgeted for any serious review times. Reviewing minor changes can be quick but if it's entirely new code, it needs to be reviewed in how it interacts with it's interfaced components and that add significant time to figure out if it's working properly or not.

      And that my gov't contractor employer has this ridiculous 'every line of code must be r

    • I commit almost hourly, reviews are for when the team needs to understand what's going on in the code.

      In the "serial monogamy" relationship that developers have with code around here, divorce and re-marriage time is usually when a review is appropriate.

  • by xxxJonBoyxxx ( 565205 ) on Friday July 08, 2011 @09:56AM (#36694738)

    Perhaps peer code reviews were less important 30 years ago. The constraints around running code were much tighter. There were fewer layers in the stack where bugs could lurk. Memory was scarce, instruction sets were limited. Computer applications were like my 1973 AMC Gremlin: fewer moving parts, so it was easier to see which parts weren't working correctly and predict when they would fail.

    Welcome to the industry, Chris. This might have been the funniest thing I read all week.

  • by jeff4747 ( 256583 ) on Friday July 08, 2011 @10:00AM (#36694788)

    I'm too busy for code reviews.

    Development schedules are almost always ridiculously compressed. There's no time in the schedule for code reviews. Just like there's no time for thorough testing.

    But the software only has to be 'good enough' for people to buy it, so there's no ammunition for developers to use to get a better schedule.

    • In the time we all spend reviewing my code, we could have each fixed separate bugs in the software or completed a new feature. Not only does the code review practically halve my productivity, it halves everyone else's.

      • Re:this (Score:5, Informative)

        by Omnifarious ( 11933 ) * <{gro.suoirafinmo} {ta} {hsals-cire}> on Friday July 08, 2011 @11:12AM (#36695940) Homepage Journal

        I've caught bugs in code review that would never in a million years have been caught by testing. The customer would've encountered them sporadically, and in situations in which they'd come to rely on the broken behavior working implicitly. The would've submitted a bug report and we would've been unable to replicate the problem in house. The customer's opinion of us would be lowered, and we'd have all these bug reports in our system that nobody could figure out.

        That is, until someone reviewed the code and discovered the bug.

        So, basically what you're doing is robbing Peter to pay Paul. By not doing the reviews you end up with less time in the long run, not more.

        • Re: (Score:3, Insightful)

          by drpentode ( 586437 )
          Finding a bug in your own code costs one of unit of work. QA finding a bug in your work costs 10 units of work. Having a customer find a bug in your code costs 100 units of work. Doing code reviews increases efficiency. Plus, you get to both teach and learn from your peers.
          • Re:this (Score:4, Insightful)

            by rmstar ( 114746 ) on Friday July 08, 2011 @04:43PM (#36700014)

            Finding a bug in your own code costs one of unit of work. QA finding a bug in your work costs 10 units of work. Having a customer find a bug in your code costs 100 units of work. Doing code reviews increases efficiency. Plus, you get to both teach and learn from your peers.

            That's all nice and dandy, but the problem is that in reality it still makes sense sometimes to produce and sell buggy software. This is so for a variety of reasons. Amongst them the fact that actually pulling off a business is a very multifacetic thing, and the product is only one of many aspects. In many cases, the cost of making a good product is simply prohibitive when viewed in the grand scheme of things.

            Reality is messy.

    • >

      But the software only has to be 'good enough' for people to buy it, so there's no ammunition for developers to use to get a better schedule.

      This is why "life safety" industries have quality standards that require documentation of design controls, verification and validation activities.

      Thing is, most industries have "fatal accidents," where the business itself is killed by relying on a bunch of technology that nobody really understands. Capitalistic evolution hasn't had enough generations of software based companies for the "fit and nimble" mammals to take down the dinosaurs yet.

    • by mcmonkey ( 96054 )

      I'm too busy for code reviews.

      And yet I'm guessing there's time to it right the second time. How much time is spent on patches and bug fixes?

      Maybe if you had code reviews, you'd be less busy.

  • Code reviews are certainly not a magic bullet. Maybe they are applicable to a situation, maybe not.

    Code reviewers are human, so are subject to all the potential problems there. I've seen code quality dragged down by code reviews because the reviewers either could not or chose not to (for various reasons, including inter-department politics) to understand a piece of code (even when it could be shown that it was the 'standard' or best current practice solution.) Ultimately the code was degraded down to the

  • by GreyWolf3000 ( 468618 ) on Friday July 08, 2011 @10:04AM (#36694854) Journal

    Because code reviews end up being an enabler for OCD coworkers to tell me how to indent.

    • by syntap ( 242090 )

      Because code reviews end up being an enabler for OCD coworkers to tell me how to indent.

      This.

      Conflicts in particular coding styles can easily derail a code review that is supposed to review for security and bugs into how something could be made minimally more efficient while eliminating overall code readability/maintainability. And there is always the guy who never comments his code, the peer reviewers take time to read through it trying to figure out what he is doing, provide comments and include a note t

      • I've almost lunged across a transatlantic gap and strangled someone through a phone line because they indicated I needed to rewrite my entire program to encode the names of the design patterns I was using into my object names and methods, or they would block my submission. This from a fellow who bulk submitted about 30 changes resulting in the build being broken for a week and a half. ...
        That said, there IS a place for it. Code architecture is somewhat subjective, and two people may come up with a more ef

  • by johnlcallaway ( 165670 ) on Friday July 08, 2011 @10:04AM (#36694858)
    If I had all the time in the world, I would test more thoroughly, and do more post-production audits too.

    But I don't have all the time in the world, so I do what I can given the time that I've been given to get work done. I consult my peers often enough that reviewing all the code all the time is a waste of both my time and theirs.

    It shouldn't be necessary to review every scrap of code. I think I've written enough that I have the basics down pretty well. And when I get into new subjects, I always talk it over with someone to see if we can find an optimal way of doing something.

    If two people have already agreed on the best way to code a specific task, having them review each others work all the time is a waste of time.
    • But I don't have all the time in the world, so I do what I can given the time that I've been given to get work done.

      So you, like me, are posting on /. at 11AM EST.

  • by rtilghman ( 736281 ) on Friday July 08, 2011 @10:06AM (#36694874)

    The problem is that the need for code reviews is driven by lax, sloppy developers who don't see regression testing as a requirement, and who foist crappy, untested code that, in many cases, they haven't even tested.

    As a consulting exec (experience side) who oversees software delivery I can't even begin to express the stunning crap that I see developers submit for "qa/review". Crap that doesn't even WORK correctly in the first place is submitted for testing, with the QA feedback often "Does not work". Aside from the hours of UE and QA resources this burns with useless testing, it highlights what I think is both an increasing lack of accountability and a lack of professionalism within the development community in general.

    What's driving this I have no idea... less formal CS training? Looser languages? Web-centric apps? Lower end standards? Higher demand = more crappy resources? Whatever it is I'm seeing it everywhere, and it's driving me nuts. The lack of an appreciate for regression testing is absolutely insane... code reviews are just symptomatic of a larger problem, which is a lack of quality and skill.

    -rt

    • The problem is that the need for code reviews is driven by lax, sloppy developers who don't see regression testing as a requirement, and who foist crappy, untested code that, in many cases, they haven't even tested.

      Code review and regression testing are separate animals.

      Code review is an application of the hive mind to the problem at hand, and an exchange of knowledge among the worker bees which can, in an ideal world, lead to better productivity for everyone after the meeting.

      Regression testing is an (imperfect) attempt to be sure that requirements are met.

      The two are complimentary, both can be misused, abused, and turned into a mockery of what they should be.

  • Hemedinger says, they also 'improve quality, ensure continuity, and keep it fresh. Who can argue against that?'"

    You mean besides the hordes of contrarian Slashdotters looking for the word Insightful to appear next to their posts? Heh.

    Well, I wouldn't bother trying to argue with that. I know that I'm more apt to design and implement my scripts a little more thoughtfully when I know somebody else is likely to pick them up and work on them. Unless you suffer from 'code fright' it's difficult to picture a scenario where this wouldn't be preferred.

  • If you are too good for code reviews, you should still do it. It will teach your reviewers how to write better code. Yeah, that's why you should.

    • If you are too good for code reviews, you should still do it. It will teach your reviewers how to write better code. Yeah, that's why you should.

      I once ran into a really excellent programmer, much more clever than I am. Where I would compare a pointer to NULL by writing "if (p == NULL)" he really preferred "if (! p)" because it demonstrated his superior intelligence.

      One day he changed, for no good reason, a line "if (p != NULL)" to "if (! p)" and checked it in. Unlike every other programmer in the company, without a code review because he was an excellent programmer who didn't need code reviews.

      That line of code was only ever executed in a gra

  • by Anonymous Coward on Friday July 08, 2011 @10:08AM (#36694900)

    I tend to use code reviews as a method for showing other team members the work that I have done so they will know about the changes. I work with a large number of geographically disperse people and having 1:1 conversations about every little change can be tiresome and wasteful. Code reviews are an excellent way to get around this problem.

    Another bonus for code reviews, along the same lines - is in teaching people new to a platform about the kinds of bugs to expect when working on the platform.

    It also keeps me honest, did I just add a debug, or do I also need to add trace? If I forgot the trace, code review will pick that up.

  • by spectro ( 80839 ) on Friday July 08, 2011 @10:08AM (#36694902) Homepage

    Having another pair of eyes on your code is a very good idea but this whole concept of everybody getting together and put somebody's work to shame is bad for team dynamics and a waste of time.

    I rather have a team dynamic where any developer can pick up a task involving work on somebody else's code, this way code reviews just happen naturally.

  • Longer answer: Code Reviews are a "best practice" that really needs to be driven by management and ingrained in the culture as "one of those things that you do" like showing up on time every day (even if that time is 10am), participating in Scrum, turning in your timesheet, etc.

    If management is lax and doesn't require code reviews on a regular basis (or, even worse, does them "as needed"), then it is an awkward unpleasant process that can become counterproductive.

    So, most places I have worked have treated c

  • I've rarely seen code reviews find anything useful. They mostly turn into nit-pick sessions about naming conventions while real bugs slip right through. And they're always done at the last minute, so finding out that a developer isn't following design standards doesn't matter because it's too late to do anything about it.

    Effective code review would take a huge amount of hours and the customer just isn't going to pay for it. It ends up just a show so we can check off that box on the CMMI audit.
    • by halivar ( 535827 )

      In my shop, if a code review shows that you did not follow proper design standards, you redesign it (re-implementing, if need be). It's bitten me before, but in the long run, I'm thankful. I realize this is not SOP across the industry due to schedules and whatnot, but I think it should be.

    • I've rarely seen code reviews find anything useful. They mostly turn into nit-pick sessions about naming conventions while real bugs slip right through. And they're always done at the last minute

      Clearly, you're doing it wrong. Not saying that your particular shop is capable of doing it right, but if nobody is even trying to do it right, what do you expect?

  • there are many tools that produce better software quality, btu they all seem to be up-front, and require a large amount of programmer time. This is probably because they are produced by programmers for programmers.

    so test-driven-development is very fashionable at the moment, even though it doesn't really work well with legacy code and can take an amazing amount of time to set up for new; similarly code reviews are great, but require lots of time for a (presumably) better coder to check what you did is ok (a

  • Management usually feels code review is a waste of time. If we could make that programmer produce more new code instead of reviewing (an apparently working) code it is more efficient right?

    The benefits of code review or intangible immeasurable benefits in the future, most likely going to benefit whoever is going to be in my chair at the time. So why invest in it? That is another tack taken by managers.

    But if bugs are found, we could estimate the resources needed to fix it and expand my empire. That is a

  • I don't so much mind having my code looked at. But I don't like looking at other people's code. Does the application work? Great. I don't need to see your code then. If you're happy and the user is happy, I'm happy.

  • Nobody is so good that they should not have their code reviewed.

    If you think you are that good, this is the first sign that you shouldn't be coding. Management Needs You.

  • Are you working in an environment in which you're the only developer working on your particular piece of code and everyone else has their own little fiefdom as well, or are you working in a more collaborative environment, such as an Agile/Scrum/whatever environment that practices paired-programming, collective code ownership, and TDD? I've worked in both (I'm currently in an Agile environment and never plan to go back to the "old" way of doing things in case you're wondering where my bias lays.), and I gott

  • The value of code reviews depends on the motivation behind it. If it's for mutual education, quality control, style conformity, etc., then it's a worthwhile venture. If, however, it's sole purpose is for the jackass-with-a-Doctor's-degree to tell me that I should abandon C and instead do my embedded development in C++, making everything object oriented (among other bloated, touchy-feely bullshit)... you can kiss my ass.
  • the amount of craptacular software that is out there, both free and paid, I would say the answer is yes.

    Between the exhaustively documented bugs in Civ and AC (to name two games from the same person/group), to the WTF? in many open source packages, coders, as a group, believe they are too good for code review.

  • Code reviews come from a time when there were few (or no) alternatives for checking that code does what you think it should, as opposed to merely compiling which was often as far as software testing went. It also comes from a time when budgets were huge, software was EXPECTED to take forever to develop and nobody minded (too much) when it did.

    Now things have changed. To expect people to find the time in their schedules to try and understand the sofwtare that one of their colleagues has written is impracti

  • Code reviews are good for finding bugs and - if you happen to have one - confirming to a coding style guide. This means that we'll be more likely to find bugs, and the code will be potentially easier to maintain. Seems like it's an easy win, at the cost of some small amount of dev time.

    However, there's a big list of detriments.

    Let's take the average developer, who stereotypically has a social coping mechanism that is ... shall we say off-by-one. Take the thing he does well, and subject it to criticism ev

    • Code reviews are good for finding bugs and - if you happen to have one - confirming to a coding style guide.

      I used to think style guides were a "Good Idea (TM)," but, then, I started getting lazy and importing other people's source code. Big chunks of it. From within the company, from LGPL sources, etc., and, guess what? Reformatting all that code to fit a style guide is a colossal waste of time.

      I actually like the "personal style" that we use here, every programmer has their own preferred way of writing their C code. As a programmer, you should be able to decipher code regardless of the style, so, now, when

  • All these things are good. Unit tests, code reviews...

    However, they all come with conditions to make them useful.

    Unit tests for example require clean abstracted code to be useful... and they cannot be used to test integration and communication without significant effort. My own view is the primary success of unit testing is that it forces developers to write well abstracted structured code... not in the actual benefits of testing units.

    Code reviews require skilled people with good knowledge of the code to

  • The secret to code reviewing is very simple: Don't be a Dick (tm). When you use Don't be a Dick (tm) when reviewing code, the reviewee will be more inclined to respect your opinion, to see his code objectively, and to incorporate your advice in the future! Of course, and entire team using Don't be a Dick (tm) will be even more effective! Get Don't be a Dick (tm) now, and start not being a dick today!
    • The secret to code reviewing is very simple: Don't be a Dick (tm).

      So very true, but since virtually all programmers are male....

  • Unfortunately, we stopped having them due to a reduced group size. Also, last time we had it, the arrogant snob being reviewed fought every single optimization suggestion from the team members and didn't put in any. Then-again, due to the eventual group size reduction, I eventually implemented all the optimizations and his code runs much better now!
  • I seriously wonder how many peopel ehre know what code reviews are about?
    a) Drop your ego. You are not gods gift to programming, and you can always improve.
    b) Many eyes make for shallow bugs
    c) It ensures there is a control on creep.
    d) It provides good and more accurate feed back regarding scope.
    e) It lets coders know if their code is understandably
    f) It bring Jr. Coders up to speed faster and increases their skill
    g) It lets Jr. Coders inform Gray beards of new techniques.
    h) It ensures people aren't slacking

  • by Mr. McGibby ( 41471 ) on Friday July 08, 2011 @10:50AM (#36695550) Homepage Journal

    If you use some software, then the process is MUCH easier. http://www.reviewboard.org/ [reviewboard.org]

  • by npsimons ( 32752 ) * on Friday July 08, 2011 @10:59AM (#36695722) Homepage Journal

    I *wish* I had code reviews! Granted, I would also like to choose my reviewers, or at least set a minimum bar ("first, write the FizzBuzz program in the language that the code we are reviewing was written in"). I can understand not wanting to subject yourself to people who really shouldn't be reviewing code (one comic I saw had a "reviewer" stating "all those curly brackets and semicolons really ruin the feng shui of the code"). And maybe some (very rare) people don't need code reviews (I suspect that most people would get more out of reviewing Ken Thompson's code than Ken Thompson would get in return; in other words, learn from the greats). But most code seriously needs to be reviewed. Just drop the ego, get over yourself, and set some standards for reviewers and coding style, then review.

  • We already have this for all release streams. Only one person (Karolin) the release manager can commit to a release stream - all other changes have to be associated with an open bug report and go through at least one code review before going in.

    Master branch is still more open for rapid development, but we're moving in the direction of more and more code reviews.

    I don't like doing code reviews, but I like having my code reviewed as other people find my bugs :-).

    Jeremy.

  • by hsthompson69 ( 1674722 ) on Friday July 08, 2011 @11:51AM (#36696564)

    ...then continuous code reviews are great. Keep two sets of eyeballs on the code *as you write it*. Yes, the antisocial archetype hates pair programming, but it simply works, even if it is uncomfortable (as code reviews often are).

    The real problem with formal code reviews is that all too often, the reviewers are so distant from the day-to-day issues in the code they can't to much more than give superficial suggestions ("I don't like how you named this variable", or "we need more comments here"). Arguably, to do a truly thorough code review, you'd have to spend *at least as much* time prepping for the code review as it took for the coder to put it together in the first place...quite probably several times more.

    If they really wanted to make them more useful, though, code reviews should be conducted on running code, and any review item should be immediately fixed in the code, and the whole regression suite of tests run in front of all the reviewers.

  • by Sarusa ( 104047 ) on Friday July 08, 2011 @02:48PM (#36698882)

    Rather, almost everyone loves the idea ('oh yeah, we should do that'), but as soon as you attach the time/budget cost to it they decide it wasn't that important anyhow.

    And they are extremely time intensive. Coders need to walk through the code line by line in advance for the review to be useful, then there's the drawn out meeting itself, and usually a manager wants to be there which adds more budget and slows things down with useless questions, then the writeup.

    Ah, you say, but it will save time in the long run, because it takes less time to track the bugs down now rather than one at a time later. But (almost) nobody is willing to trade possible future time loss against significant right now time loss.

  • by golodh ( 893453 ) on Friday July 08, 2011 @05:50PM (#36700632)
    Whether a code review is useful all depends on who's doing it.

    A nice caricature can be found here: http://www.jsquared.co.uk/jennyl/verity.htm [jsquared.co.uk]

    Don't believe that his sort of response during code reviews is all that uncommon.

A complex system that works is invariably found to have evolved from a simple system that works.

Working...