launchpad developmentThis page needs to be cleaned up
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
UserInterfaceChecklist - low-level Web design and e-mail design expectations
CodeReviewChecklist - checklist that should be pass by all reviewed code
TipsForReviewers
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.
