Jump to: navigation, search

Difference between revisions of "Solum/Contributing"

m (Review Guidelines)
(Set up your local branch)
 
(11 intermediate revisions by 4 users not shown)
Line 2: Line 2:
  
 
= Before You Begin =
 
= Before You Begin =
The configuration of the [https://github.com/stackforge/solum Solum StackForge Repo] requires that you have an [[How_To_Contribute#Contributors_License_Agreement|OpenStack CLA]].  
+
To familiarize yourself with Solum, try it out using the information in our [https://solum.readthedocs.org/en/latest/getting_started/ Getting Started Guide]. When you are ready to start contributing, you will need to execute an [[How_To_Contribute#Contributors_License_Agreement|OpenStack CLA]]. This is required before you can submit reviews to our [https://github.com/openstack/solum Solum Repo]. For information about how prepare for contribution, please consult the [[How_To_Contribute |How To Contribute]] wiki page.
For information about how prepare for contribution, please consult the [[How_To_Contribute |How To Contribute]] wiki page.
 
  
 
== Learn About Gerrit ==
 
== Learn About Gerrit ==
Be sure to read the [[Gerrit_Workflow|Gerrit Workflow]] wiki page for information about how to submit your commit for review so it can be merged into the Solum code base.
+
Be sure to read the [http://docs.openstack.org/infra/manual/developers.html Developer's Guide] for information about how to submit your commit for review so it can be merged into the Solum code base.
  
 
== Setting up your git review settings ==
 
== Setting up your git review settings ==
Line 21: Line 20:
 
   pip install git-review
 
   pip install git-review
  
There are other installation options detailed in the [[Gerrit_Workflow#Git_Review_Installation|Installation Instructions]]. You can now check out the Solum code and begin working on it:
+
There are other installation options detailed in the [http://docs.openstack.org/infra/manual/developers.html Developer's Guide]. You can now check out the Solum code and begin working on it:
 
 
  
 
= Your first commit =
 
= Your first commit =
Line 29: Line 27:
  
  
   git clone git://git.openstack.org/stackforge/solum
+
   git clone git://git.openstack.org/openstack/solum
 
   cd solum
 
   cd solum
 
   git checkout -b [branch name]
 
   git checkout -b [branch name]
 
   git review -s
 
   git review -s
  
== Modify CONTRIBUTORS file ==  
+
== Write some awesome code ==
  
Open the file for edit, add your name and email address to the list and save the file.
+
At this point can write your code and push it to Gerrit for review.
  
  vi CONTRIBUTORS.rst
+
   git add <list of files you added/changed>
 
 
== Create a draft review ==
 
 
 
   git add CONTRIBUTORS.rst
 
 
   git commit -a
 
   git commit -a
 
   git review -v --draft
 
   git review -v --draft
  
At this point if all has been successful it should provide you with a review.openstack.org URL to track your changeFollow that URL and "Sign In" and you should see your change.
+
Once you are happy with your code and want it to be reviewed you want to convert it from a Draft.  "Sign In" at https://review.openstack.org/ and after verifying the review yourself hit the "Publish" button on the page.
 
 
== Write some awesome code ==
 
 
 
At this point can write your code and push it up to your review.   You will want to amend your changes to the original commit ( reduce noise )
 
  
  git add <list of files you added/changed>
+
If you know you are ready for others to review your code, you can skip the draft step and just do:
  git commit --amend
+
git review -v
  git review -v
 
  
Once you are happy with your code and want it to be reviewed you want to convert it from a Draft.  "Sign In" at https://review.openstack.org/ and after verifying the review yourself hit the "Publish" button on the page.
+
If you want to revise your patchset in the review system in response to feedback, make your changes, then use:
 +
git commit -a --amend
 +
git review -v
  
 
Upon approval of the review your code will be automatically merged.
 
Upon approval of the review your code will be automatically merged.
Line 75: Line 66:
  
 
== Review Guidelines ==
 
== Review Guidelines ==
* For Approval, two core reviewers should supply a +2.  
+
=== Code Approval for Merge ===
 +
* For Approval, two core reviewers shall supply a <code>+2</code>.  
 +
=== Continuing Someone Else's Contribution ===
 
* If a patch submitted by one contributor is picked up and completed by another contributor, [http://www.mail-archive.com/openstack-dev@lists.openstack.org/msg05998.html special handling] of the resolution should be used.
 
* If a patch submitted by one contributor is picked up and completed by another contributor, [http://www.mail-archive.com/openstack-dev@lists.openstack.org/msg05998.html special handling] of the resolution should be used.
 +
=== Advice for Reviewers ===
 +
* A <code>-1</code> vote is an opportunity to make our code better before it is merged. Please do your best to make helpful, actionable -1 votes.
 +
* Avoid the temptation to blindly <code>+1</code> code without reviewing it in sufficient detail to form an opinion.
 +
* When voting <code>-1</code> on a patch, it means that you want the submitter to make a revision in accordance with your feedback before core reviewers should consider this code for merge.
 +
* If you ask a question, you should vote <code>0</code> unless you anticipate that the answer to that question is likely to cause you to vote against the patch without further revisions.
 +
* If you use a <code>-1</code> vote for a question, and the contributor answers the question, please respond acknowledging the question. Either change your vote or follow up with additional rationale for why this should remain a <code>-1</code> comment.
 +
* A <code>-2</code> vote is a veto by a single core reviewer. It is sticky. That means that even if you revise your patch, that vote will persist. To allow your patch to merge, that same reviewer must clear the <code>-2</code> vote first. This vote is used when you have contributed something that is not in alignment with the current project vision, or is implemented in a way that can not be accepted. For example, security concerns that a core reviewer wants to individually re-evaluate before allowing the contribution to continue. It can also be used as a way to halt further gate testing of a patch, if something is included that may break the gate. It works even after a <code>2*+2,+A</code> approval for merge, but before the patch reaches MERGED state.
 +
* To avoid a <code>-2</code> vote, discuss your plans with the development team prior to writing code, and post a WIP (workflow-1) patch while you are working on it, and ask for input before you submit it for merge review.
 +
 +
= Testing =
 +
See our [[Solum/Testing]] wiki.

Latest revision as of 18:50, 9 October 2015

Before You Begin

To familiarize yourself with Solum, try it out using the information in our Getting Started Guide. When you are ready to start contributing, you will need to execute an OpenStack CLA. This is required before you can submit reviews to our Solum Repo. For information about how prepare for contribution, please consult the How To Contribute wiki page.

Learn About Gerrit

Be sure to read the Developer's Guide for information about how to submit your commit for review so it can be merged into the Solum code base.

Setting up your git review settings

 git config --global user.name "Firstname Lastname"
 git config --global user.email "your_email@youremail.com"
 git config --global gitreview.username "your_launchpad_username"

To check your git configuration:

 git config --list

Installing git-review

On Ubuntu, MacOSx, or most other Unix-like systems, it is as simple as:

 pip install git-review

There are other installation options detailed in the Developer's Guide. You can now check out the Solum code and begin working on it:

Your first commit

Set up your local branch

 git clone git://git.openstack.org/openstack/solum
 cd solum
 git checkout -b [branch name]
 git review -s

Write some awesome code

At this point can write your code and push it to Gerrit for review.

 git add <list of files you added/changed>
 git commit -a
 git review -v --draft

Once you are happy with your code and want it to be reviewed you want to convert it from a Draft. "Sign In" at https://review.openstack.org/ and after verifying the review yourself hit the "Publish" button on the page.

If you know you are ready for others to review your code, you can skip the draft step and just do:

git review -v

If you want to revise your patchset in the review system in response to feedback, make your changes, then use:

git commit -a --amend
git review -v

Upon approval of the review your code will be automatically merged.

Reviews

The OpenStack CI system uses the concept of core reviewers. These are individuals who have consistently reviewed code for the project, and helped over a considerable period of time to improve the quality and consistency of what we merge into the code base. Project contributors will feel that this reviewer is a positive influence on the team and that they maintain the values and traditions of the OpenStack development community.

Policies

Existing core reviewers may nominate new ones in an ML thread. Consent among the current reviewers shall result in the declaration of the new core reviewer by the PTL. Lack of unanimous consent shall be carefully considered, and a final decision informed by input from from active team members shall be made by the PTL. Core reviewers who are judged by their peers in the core review group to fall short of the expectations for contribution of a core reviewer may be nominated for return to regular reviewer status.

The current Gerrit policy is:

label-Code-Review = -2..+2 group solum-core
label-Approved = +0..+1 group solum-core

Patches require a core reviewer to mark a review as "Approved" before they are merged.

Review Guidelines

Code Approval for Merge

  • For Approval, two core reviewers shall supply a +2.

Continuing Someone Else's Contribution

  • If a patch submitted by one contributor is picked up and completed by another contributor, special handling of the resolution should be used.

Advice for Reviewers

  • A -1 vote is an opportunity to make our code better before it is merged. Please do your best to make helpful, actionable -1 votes.
  • Avoid the temptation to blindly +1 code without reviewing it in sufficient detail to form an opinion.
  • When voting -1 on a patch, it means that you want the submitter to make a revision in accordance with your feedback before core reviewers should consider this code for merge.
  • If you ask a question, you should vote 0 unless you anticipate that the answer to that question is likely to cause you to vote against the patch without further revisions.
  • If you use a -1 vote for a question, and the contributor answers the question, please respond acknowledging the question. Either change your vote or follow up with additional rationale for why this should remain a -1 comment.
  • A -2 vote is a veto by a single core reviewer. It is sticky. That means that even if you revise your patch, that vote will persist. To allow your patch to merge, that same reviewer must clear the -2 vote first. This vote is used when you have contributed something that is not in alignment with the current project vision, or is implemented in a way that can not be accepted. For example, security concerns that a core reviewer wants to individually re-evaluate before allowing the contribution to continue. It can also be used as a way to halt further gate testing of a patch, if something is included that may break the gate. It works even after a 2*+2,+A approval for merge, but before the patch reaches MERGED state.
  • To avoid a -2 vote, discuss your plans with the development team prior to writing code, and post a WIP (workflow-1) patch while you are working on it, and ask for input before you submit it for merge review.

Testing

See our Solum/Testing wiki.