Beefy Boxes and Bandwidth Generously Provided by pair Networks
good chemistry is complicated,
and a little bit messy -LW

Comment on

( #3333=superdoc: print w/replies, xml ) Need Help??
A code review is a meeting. All meetings have inputs, attendees, agendas, and outputs. In the case of a code review, it could look something like:
  • Inputs: Code to be reviewed; Specifications for said code
  • Attendees: Author, moderator, reviewers (no, the author cannot moderate the review of his code)
  • Agenda: Review the code
  • Outputs: Suggestions for improvements

So, document that. A couple thoughts:

  • Some people talk about non-meeting code reviews, where people review the code at their leisure and give feedback via email. This system sucks because there's no impetus to do anything.
  • Unless you have management backing, you will not get people to give the amount of time necessary to review code properly. This breaks down as so (for roughly 100-200 lines of code spanning 2-3 functions):
    • 30min - 1h reading and understanding specifications (you have specs, right?)
    • 30min - 2h reading and understanding the context of the code (calling code, called code, environment, etc)
    • 1h - 3h reading and understanding the code in question
    • 1h for the actual meeting (Code reviews should never go past 1h. Anything more and the code needs rewritten or chunked better.)
    As you can see, this could be up to an entire workday per reviewer. And this doesn't count any followup reviews should the code in question need to be rewritten.

    Now, this will get better over time, but you should expect that in the worst case (and we all plan for the worst case, right?).

  • Expect a lot of egos, especially at first. It's difficult to hear someone you think is your inferior tell you what's wrong with your baby, particularly if they're right. This is why the author cannot be the moderator.
  • Don't even think you're going to teach the reviewers with your code. That's arrogance at its finest.

My criteria for good software:
  1. Does it work?
  2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?

In reply to Re: How should I do (and document) effective semi-formal code review? by dragonchild
in thread How should I do (and document) effective semi-formal code review? by radiantmatrix

Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":

  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.
  • Log In?

    What's my password?
    Create A New User
    and all is quiet...

    How do I use this? | Other CB clients
    Other Users?
    Others wandering the Monastery: (10)
    As of 2018-06-21 14:51 GMT
    Find Nodes?
      Voting Booth?
      Should cpanminus be part of the standard Perl release?

      Results (118 votes). Check out past polls.