We review code patches to ensure that submissions are relevant, good quality and following the coding guidelines and best practices.
Who should submit patches?
Contributors do not have commit privileges to the trunk and are required to submit patches through the review board.
Committers have commit privileges to the trunk but may choose to still submit the patches on review board if they want to solicit feedback from the community or specific persons.
How to submit a new patch?
Apache communities use Review Board Tool to manage code reviews. If you have not setup your account yet you will need to register first.
Review Board is used by most Apache Projects and shows reviews for all of these projects. Review Board uses the term "Groups" for the projects. If you are only interested in Apache CloudStack then you should update your preferences and select 'cloudstack' in Groups section on preferences page.
To submit a new patch:
- Click on the New Review Request link.
- 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
- In "Groups" auto-fill text field specify "cloudstack"
- In "People" auto-fill text field type in the name of persons who should be reviewing the patch. You can 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
- In "Branch" text field specify the branches your patch should be submitted to.
- If your patch fixes a defect specify the cloudstack jira bug id in "Bugs" text field.
- In "Description" field add a brief description of what your patch does
- In "Testing" field specify what tests were performed
- 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 evaluate the patch and provide feedback to the submitter. If the reviewer is satisfied with the patch, they should mark the patch as approved by clicking on "Ship It" button.
Now this is where the reviewers need to supplement the workflow by tagging as comment
- "Ship It: Not committed, Pending review from $Reviewer_Name" : Reviewers should tag the review as pending another reviewer if they are 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 reviewer can ask another reviewer/ committer to commit the patch.
- "Ship It: Committed $commit_id" : Reviewer has reviewed the patch, is satisfied, 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
As a Contributor
- 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
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.