You are currently viewing a snapshot of www.mozilla.org taken on April 21, 2008. Most of this content is highly out of date (some pages haven't been updated since the project began in 1998) and exists for historical purposes only. If there are any pages on this archive site that you think should be added back to www.mozilla.org, please file a bug.

Skip to main content

Seamonkey Code Reviewer's Guide

If you code review someone else's changes, you are on the hook with them to see that those changes are good. Here is a sampling of questions you should be asking if you are asked to review someone else's code.

  1. Am I the right person to be reviewing this code?
  2. What was the original problem you were trying to solve or bug you were trying to fix?
    1. was there a bug report on the problem?
    2. can we look at the bug report?
    3. did the nature or scope of the of the problem evolve during the investigation?  how?
    4. were there any trade-off's (e.g. size/speed, UI completeness/simplicity) that you considered or made in constructing the fix?
  3. Show me the code
    • Let me see the new code
    • Let me see the diffs
  4. Tell me how you have tested the code
    • Show me a simple test case
    • Show me a bigger test case that shows you haven't broken anything
    • What are the error conditions this code might experience?
  5. What could this change have possibly broken?
    • What effects might this change have on size, speed, or memory use?
    • Have you checked for leaks or premature free memory conditions?
    • Have you checked for race conditions, error conditions, or infinite loops?
    • Where are all the places this function is called from?
    • Could anyone or anything depend on the old broken behavior?
    • Does the change consider platform issues like 64 bit ints, does this code work?
    • Do the changes affect a single platform, or are they cross-platform?
    • What other machines have you compiled and tested this code on?
    • Have you check the code for possible compiler quirks
  6. Is this the right time to be making this change?
    • Is this a fix to a fix, or in a high risk area?
    • Is there a simpler change which solves the problem in all the important cases?
    • Is there another release coming where this change would be more appropriate?

Remember, a code reviewer is not a friend. A good code reviewer can feel like your worst enemy.