Review instructions/policy/workflow

• Things you can do, as a reviewer, to avoid round trips
  • Include diff chunks for context
  • Do not make reference to line numbers, as diffs are a moving target
  • Always include the file name in diff chunks
• Please subscribe to these wiki pages (they are low volume, but changes matter):

  • PreMergeReviews - why and how we are doing this.

  • WorkingWithReviews - the workflow from the perspective of a coder wanting to land a branch - what you can expect from them, and what they are expecting from you.

  • LaunchpadHackingFAQ - the instructions here document the expectations we are setting for code that comes up for review - even if we have issues and questions not expressed there, all code that is oked should pass all of the criterion in that FAQ

  • UIReviews - low-level Web design and e-mail design expectations

  • CodeReviewChecklist - checklist that should be pass by all reviewed code

  • TipsForReviewers

  • Python style guide

  • JavaScriptReviewNotes

  • TestsStyleGuide

  • ReviewerMeetingAgenda - when and where reviewers meet on irc.

• Changes to security.cfg can be made even during DB freeze. This runs the risk of causing oopses on edge.launchpad.net, if edge requires a permission that has not been granted to the production DB. So if you are landing a security.cfg change while the DB is frozen, you need to email stub, mthaddon, CC: launchpad@ asking them to apply the manual change to jubany as well. Reviewers should remind about this when seeing a security.cfg change.

Suggested reading

• http://c2.com/cgi/wiki?CodeReviewPatterns has a wealth of information about how other people do code reviews, and it is fundamentally good stuff - have a read through at some point.