Jump to: navigation, search

Difference between revisions of "Sahara/ReviewChecklist"

 
Line 1: Line 1:
This page includes a list of things that reviewers should keep in mind when reviewing patches to [[Sahara]] project, it's based on the OpenStack's [http://docs.openstack.org/infra/manual/developers.html#peer-review ReviewChecklist].
+
This page includes a list of things that reviewers should keep in mind when reviewing patches to [[Sahara]] project, it's based on the OpenStack's [http://docs.openstack.org/infra/manual/developers.html#peer-review Review Checklist].
  
 
= Common Review Checklist =
 
= Common Review Checklist =

Latest revision as of 18:59, 4 December 2014

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

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 Sahara/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.