Jump to: navigation, search

Difference between revisions of "Sahara/ReviewChecklist"

m (Sergey Lukjanov moved page Savanna/ReviewChecklist to Sahara/ReviewChecklist: Savanna project was renamed due to the trademark issues.)
(No difference)

Revision as of 15:41, 7 March 2014

This page includes a list of things that reviewers should keep in mind when reviewing patches to Savanna project, it's based on the OpenStack's ReviewChecklist.

Common Review Checklist

  1. The code should comply with everything in HACKING.
  2. The code should be 'pythonic' and look like the code around it, to make the code more uniform and easier to read
  3. When adding a new file:
    1. If it's not installed by setup.py, but should be included in the tarball, be sure to add it to `MANIFEST.in`.
  4. Commit Message and Change break-up
    1. Follow the advice of Savanna/GitCommits
    2. If the patch fixes a bug, it should reference a bug report
    3. If the patch implements a feature, it should reference a blueprint. The blueprint should be approved before the patch is merged
  5. When to approve:
    1. Most of the patches need two +2s before being approved, please, don't hurry to make other contributors able to look at the patch
    2. If a patch has already been approved but requires a trivial/simple rebase to merge, you do not have to wait for a second +2, since the patch has already had two +2s

Notes for Non-Core Developers

  • When you are reviewing, you may notice a review that has several +1's from other reviewers, passes the functional tests, etc. but the code still has not been merged. As only core developers can approve code for merging, you can help things along by getting a core developer's attention and letting them know there is a review with lots of positive reviews and needs final approval.