Page History

<DRAFT WORK IN PROGRESS>

We review code patches to ensure that submissions are relevant, good quality and following the coding guidelines and best practices.

...

1. Click on the New Review Request link.
2. Select "cloudstack-git" in the Repository dropdown, upload your diff and click "Create Review Request" to get to next page where you will enter more details in several fields
3. In "Groups" auto-fill text field specify "cloudstack"
4. In "People" auto-fill text field type in the name of persons who should be reviewing the patch. You can chose choose more than one person to review your patch. If you are not sure who should be reviewing your patch please check CloudStack Maintainers List By Components
5. In "Branch" text field specify the branches your patch should be submitted to.
6. If your patch fixes a defect specify the cloudstack jira bug id in "Bugs" text field.
7. In "Description" field add a brief description of what your patch does
8. In "Testing" field specify what tests were performed
9. Click "Publish" button to submit your patch

...

ReviewBoard tool is a simple tool and has only 3 states ("Pending", "Submitted", "Discarded") and relies on humans to complete the workflow. Once review is posted it is in "Pending" state. The reviewers get notified by email. The reviewers evaluates evaluate the patch and provides provide feedback to the submitter. If the reviewer is satisfied with the patch, they should mark the patch as approved by cliking clicking on "Ship It" button.

Now this is where the reviewers need to supplement the workflow by tagging as comment

1. "Ship It: Not commitedcommitted, Pending review from $Reviewer_Name" : You Reviewers should tag the review as pending another reviewer if you they are satisified satisfied with the review but want another pair of eyes to review it. There is ambiguity in which reviewer should commit the patch when there are multiple reviewers. The last person reviewing the patch and giving a "Ship It" should commit the patch but you reviewer can ask another reviewer/ committer to commit the patch.
2. "Ship It: Committed $commit_id" : Reviewer has reviewed the patch, is satisifiedsatisfied, has committed the patch in appropriate branch". The commit id is needed to confirm that the patch has been committed.

The submitter completes the work flow once the patch has been committed by changing the state to "Submitted" manually Image Added

Responsibilities

...

• Make sure your patches apply cleanly when posted for review
• Test that the the patch actually works and indicate what testing has been performed. e.g. Documentation patches should be built with Publican before submitting a patch.  A good reference for what we mean by actually works is to look at the project's Branch Merge Expectations page.
• Respond to review comments and provide an updated patch incorporating the feedback
• Make sure to follow up on your patches. If they do not get reviewed and committed within 3 days, send an email to reviewers and dev mailing list.
• Do not mark your patch as "Ship It" yourself, that is expected to be marked by reviewer if he/she is satisifed with your patch

As a Committer

• Respond to reviews timely when you are asked to review a patch
• Follow up with submitter if they do not respond to your comments
• It’s not enough to say “Ship It”. You have to actually apply the patch. Once you have applied the patch update the review with commit details.
• Actively look out for any reviews that are pending over a week and review it if falls in your area of expertise, if not ask another committer who is more appropriate to review the change

Acknowledging contribution

Since we use git as our version control it is encouraged that contributors use git format-patch to generate their patches.  This allows the community to attribute the code contribution to the author of the patch. It also makes it easy for the contributors to be elected as committers when the PMC notices all the valuable contributions from their patches. For patches that don't come in with author information committers are requested to amend the author information (git commit --author) before pushing to help acknowledge the contribution.