Merge "First patch and first review docs"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
80239b82ed
227
doc/source/first_patch.rst
Normal file
227
doc/source/first_patch.rst
Normal file
@ -0,0 +1,227 @@
|
|||||||
|
..
|
||||||
|
Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||||
|
not use this file except in compliance with the License. You may obtain
|
||||||
|
a copy of the License at
|
||||||
|
|
||||||
|
http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
|
||||||
|
Unless required by applicable law or agreed to in writing, software
|
||||||
|
distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||||
|
WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||||
|
License for the specific language governing permissions and limitations
|
||||||
|
under the License.
|
||||||
|
|
||||||
|
Your first patch
|
||||||
|
================
|
||||||
|
|
||||||
|
This section describes how to create your first patch and upload it to
|
||||||
|
Gerrit_ for reviewing.
|
||||||
|
|
||||||
|
|
||||||
|
Create your contributor accounts and set up your code environment
|
||||||
|
-----------------------------------------------------------------
|
||||||
|
|
||||||
|
Accounts setup
|
||||||
|
##############
|
||||||
|
|
||||||
|
You will need to create a Launchpad_ account to login to the Gerrit_ review system dashboard.
|
||||||
|
This is also useful for automatically crediting bug fixes to you when you address them
|
||||||
|
with your code commits. You will also have to sign the `Contributors License Agreement`_ and `join the OpenStack Foundation`_. It is a good idea to use the same email all of these accounts to
|
||||||
|
avoid hooks errors.
|
||||||
|
|
||||||
|
Visit the the `Gerrit Workflow's account setup`_ section in the wiki
|
||||||
|
to get more information on setting up your accounts.
|
||||||
|
|
||||||
|
.. _Launchpad: http://launchpad.net/
|
||||||
|
.. _Gerrit: http://review.openstack.org/
|
||||||
|
.. _`Contributors License Agreement`: https://wiki.openstack.org/wiki/How_To_Contribute#Contributor_License_Agreement
|
||||||
|
.. _`join the OpenStack Foundation`: http://openstack.org/join
|
||||||
|
.. _`Gerrit Workflow's account setup`: https://wiki.openstack.org/wiki/Gerrit_Workflow#Account_Setup
|
||||||
|
|
||||||
|
SSH setup
|
||||||
|
#########
|
||||||
|
|
||||||
|
You are going to need to create and upload an SSH key to Gerrit to be
|
||||||
|
able to commit changes for review. To create an SSH key::
|
||||||
|
|
||||||
|
$ ssh-keygen –t rsa
|
||||||
|
|
||||||
|
You can optionally enter a password to enchance security.
|
||||||
|
|
||||||
|
View and copy your SSH key::
|
||||||
|
|
||||||
|
$ less ~/.ssh/id_rsa.pub
|
||||||
|
|
||||||
|
Now you can `upload the SSH key to Gerrit`_.
|
||||||
|
|
||||||
|
.. _`upload the SSH key to Gerrit`: https://review.openstack.org/#/settings/ssh-keys
|
||||||
|
|
||||||
|
Git Review installation
|
||||||
|
#######################
|
||||||
|
|
||||||
|
Before you start working, make sure you have git-review installed on your system.
|
||||||
|
You can install it with the following command::
|
||||||
|
|
||||||
|
$ pip install git-review
|
||||||
|
|
||||||
|
Git-review checks if you can authenticate to Gerrit with your SSH key.
|
||||||
|
It will ask your for your username. You can configure your Gerrit username so you don't have to
|
||||||
|
keep re-entering it everytime you want to use git-review::
|
||||||
|
|
||||||
|
$ git config --global gitreview.username yourgerritusername
|
||||||
|
|
||||||
|
You can also save some time by entering your email and your name::
|
||||||
|
|
||||||
|
$ git config --global gitreview.email "yourgerritemail"
|
||||||
|
$ git config --global gitreview.name "Firstname Lastname"
|
||||||
|
|
||||||
|
|
||||||
|
You can view your Gerrit user name in the `settings page`_.
|
||||||
|
|
||||||
|
.. _`settings page`: https://review.openstack.org/#/settings/
|
||||||
|
|
||||||
|
Project setup
|
||||||
|
#############
|
||||||
|
|
||||||
|
Clone the Zaqar repository with the following git command::
|
||||||
|
|
||||||
|
$ git clone git://git.openstack.org/openstack/zaqar.git
|
||||||
|
|
||||||
|
For information on how to set up the Zaqar development environment
|
||||||
|
see :doc:`development-environment`.
|
||||||
|
|
||||||
|
Before writing code, you will have to do some configurations
|
||||||
|
to connect your local repository with Gerrit. You will only need to do this
|
||||||
|
your first time setting up the development environment.
|
||||||
|
|
||||||
|
You can set git-review to configure the project and install the gerrit change-id commit hook with the following command::
|
||||||
|
$ cd zaqar
|
||||||
|
$ git review -s
|
||||||
|
|
||||||
|
If you get the error "We don't know where your Gerrit is", you will need to
|
||||||
|
add a new git remote. The URL should be in the error message. Copy that and
|
||||||
|
create the new remote. It looks something like::
|
||||||
|
|
||||||
|
$ git remote add gerrit ssh://<username>@review.openstack.org:29418/openstack/zaqar.git
|
||||||
|
|
||||||
|
In the project directory you have a hidden .git directory and a
|
||||||
|
.gitreview file. You can view them with the following command::
|
||||||
|
|
||||||
|
$ ls -la
|
||||||
|
|
||||||
|
Making a patch
|
||||||
|
--------------
|
||||||
|
|
||||||
|
Pick or report a bug
|
||||||
|
####################
|
||||||
|
|
||||||
|
You can start tackling some bugs from the `bugs list in Launchpad`_.
|
||||||
|
If you find a bug you want to work on, assign yourself. Make sure to read
|
||||||
|
the bug report. If you need more information, ask the reporter to provide
|
||||||
|
more details through a comment on Launchpad or through IRC or email.
|
||||||
|
|
||||||
|
If you find a bug, look through Launchpad to see if it has been reported. If it hasn't, report the bug, and
|
||||||
|
ask for another developer to confirm it. You can start working on it if another developer confirms the bug.
|
||||||
|
|
||||||
|
Here are some details you might want to include when filling out a bug report:
|
||||||
|
* The release, or milestone, or commit ID corresponding to the software that you are running
|
||||||
|
* The operating system and version where you've identified the bug
|
||||||
|
* Steps to reproduce the bug, including what went wrong
|
||||||
|
* Description of the expected results instead of what you saw
|
||||||
|
* Portions of your log files so that you include only relevant excerpts
|
||||||
|
|
||||||
|
In the bug comments, you can contribute instructions on how to fix a given bug,
|
||||||
|
and set the status to "Triaged".
|
||||||
|
|
||||||
|
You can read more about `Launchpad bugs`_ in the wiki.
|
||||||
|
|
||||||
|
.. _`bugs list in Launchpad`: https://bugs.launchpad.net/zaqar
|
||||||
|
.. _`Launchpad bugs`: https://wiki.openstack.org/wiki/Bugs
|
||||||
|
|
||||||
|
Workflow
|
||||||
|
########
|
||||||
|
|
||||||
|
Make sure your repo is up to date. You can update it with the following git commands::
|
||||||
|
|
||||||
|
$ git remote update
|
||||||
|
$ git checkout master
|
||||||
|
$ git pull --ff-only origin master
|
||||||
|
|
||||||
|
Create a topic branch. You can create one with the following git command::
|
||||||
|
|
||||||
|
$ git checkout -b TOPIC-BRANCH
|
||||||
|
|
||||||
|
If you are working on a blueprint, name your topic branch bp/BLUEPRINT where BLUEPRINT
|
||||||
|
is the name of a blueprint in Launchpad (for example, "bp/authentication"). The general
|
||||||
|
convention when working on bugs is to name the branch bug/BUG-NUMBER
|
||||||
|
(for example, "bug/1234567").
|
||||||
|
|
||||||
|
Read more about the commit syntax in the `Gerrit workflow`_ wiki.
|
||||||
|
|
||||||
|
.. _`Gerrit workflow`: https://wiki.openstack.org/wiki/Gerrit_Workflow
|
||||||
|
|
||||||
|
Design principles
|
||||||
|
#################
|
||||||
|
|
||||||
|
Zaqar lives by the following design principles:
|
||||||
|
|
||||||
|
* `DRY`_
|
||||||
|
* `YAGNI`_
|
||||||
|
* `KISS`_
|
||||||
|
|
||||||
|
.. _`DRY`: https://en.wikipedia.org/wiki/Don%27t_repeat_yourself
|
||||||
|
.. _`YAGNI`: https://en.wikipedia.org/wiki/YAGNI
|
||||||
|
.. _`KISS`: https://en.wikipedia.org/wiki/KISS_principle
|
||||||
|
|
||||||
|
Try to stick to these design principles when working on your patch.
|
||||||
|
|
||||||
|
Test your code
|
||||||
|
##############
|
||||||
|
|
||||||
|
It is important to test your code and follow the python code style guidelines.
|
||||||
|
See :doc:`running_tests` for details on testing.
|
||||||
|
|
||||||
|
Submitting a patch
|
||||||
|
------------------
|
||||||
|
|
||||||
|
Once you finished coding your fix, add and commit your final changes.
|
||||||
|
Your commit message should:
|
||||||
|
|
||||||
|
* Provide a brief description of the change in the first line.
|
||||||
|
* Insert a single blank line after the first line.
|
||||||
|
* Provide a detailed description of the change in the following lines,
|
||||||
|
breaking paragraphs where needed.
|
||||||
|
* The first line should be limited to 50 characters and should not end with a period.
|
||||||
|
* Subsequent lines should be wrapped at 72 characters.
|
||||||
|
* Put the 'Change-id', 'Closes-Bug #NNNNN' and 'blueprint NNNNNNNNNNN'
|
||||||
|
lines at the very end.
|
||||||
|
|
||||||
|
Read more about `making a good commit message`_.
|
||||||
|
|
||||||
|
To submit it for review use the following git command::
|
||||||
|
|
||||||
|
$ git review
|
||||||
|
|
||||||
|
You will see the URL of your review page once it is successfully sent. You can also see your
|
||||||
|
reviews in "My Changes" in Gerrit. The first thing to watch for is a +1 in the "Verified"
|
||||||
|
column next to your patch in the server and/or client list of pending patches.
|
||||||
|
If the "Jenkins" user gives you a -1, you'll need to check the log it posts to
|
||||||
|
find out what gate test failed, update your patch, and resubmit.
|
||||||
|
|
||||||
|
You can set your patch as a "work in progress" if your patch is not ready to be merged,
|
||||||
|
but you would still like some feedback from other developers. To do this
|
||||||
|
leave a review on your patch setting "Workflow" to -1.
|
||||||
|
|
||||||
|
Once the gate has verified your patch, other Zaqar devs will take a look and submit
|
||||||
|
their comments. When you get two or more +2's from core reviewers,
|
||||||
|
the patch will be approved and merged.
|
||||||
|
|
||||||
|
Don't be discouraged if a reviewer submits their comments with a "-1".
|
||||||
|
Patches iterate through several updates and reviews before they are ready for merging.
|
||||||
|
To reply to feedback save all your comments as draft, then click on the "Review" button.
|
||||||
|
When replying to feedback, you as the patch author can use the score of "0".
|
||||||
|
The only exception to using the score of "0" is when you discover a blocking issue
|
||||||
|
and you don't want your patch to be merged. In which case, you can review your own
|
||||||
|
patch with a "-2", while you decide whether to keep, refactor, or withdraw the patch.
|
||||||
|
|
||||||
|
.. _`making a good commit message`: https://wiki.openstack.org/wiki/GitCommitMessages
|
||||||
74
doc/source/first_review.rst
Normal file
74
doc/source/first_review.rst
Normal file
@ -0,0 +1,74 @@
|
|||||||
|
..
|
||||||
|
Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||||
|
not use this file except in compliance with the License. You may obtain
|
||||||
|
a copy of the License at
|
||||||
|
|
||||||
|
http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
|
||||||
|
Unless required by applicable law or agreed to in writing, software
|
||||||
|
distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||||
|
WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||||
|
License for the specific language governing permissions and limitations
|
||||||
|
under the License.
|
||||||
|
|
||||||
|
Your first review
|
||||||
|
=================
|
||||||
|
|
||||||
|
The review stage is a very important part in the development process. Following are some of the reasons this stage is important:
|
||||||
|
|
||||||
|
* Getting other developers feedback minimizes the risk of adding
|
||||||
|
regressions to the code base and ensures the quality of the code being merged.
|
||||||
|
* Building the community encourages everyone to review code. Everyone appreciates having their code reviewed.
|
||||||
|
* Since developers are always learning from being exposed to the points of view of others, reviews help developers to improve their coding skills.
|
||||||
|
* Providing a review is a great way to become familar with the code.
|
||||||
|
|
||||||
|
Everyone is encourages to review code. You don't need to know every detail of the code base. You need to understand only what the code related to the fix does.
|
||||||
|
|
||||||
|
Step by step
|
||||||
|
------------
|
||||||
|
|
||||||
|
Go to review.openstack.org and filter by `Open Zaqar fixes`_. Select a fix from the list to review.
|
||||||
|
Try to select an easy patch for your first review. That will help you to gain some confidence.
|
||||||
|
Download the patch to your local repository and test it::
|
||||||
|
|
||||||
|
$ git review -d [review-id]
|
||||||
|
|
||||||
|
The review-id is the number in the URL (check the screenshot for more details).
|
||||||
|
|
||||||
|
Example::
|
||||||
|
|
||||||
|
$ git review -d 92979
|
||||||
|
|
||||||
|
.. image:: images/zaqar_review_id.png
|
||||||
|
:alt: Zaqar review id
|
||||||
|
|
||||||
|
This git command creates a branch with the author's name and enables you to test the patch in your local environment.
|
||||||
|
|
||||||
|
* Inspect the code. Use all of the best programming practices you know as you review the code.
|
||||||
|
* Give code location feedback. (Do you consider that some code should be better located in another place within the file, or maybe in another file? If so, suggest this in the review comment and score with a -1 if you think that it's that important.)
|
||||||
|
* Give code-style feedback. (Do you think that the code structure could be improved? Keep the DRY, YAGNI and KISS principles in mind.)
|
||||||
|
* Give grammar and orthography feedback. Many of our contributors are not native English speakers, so it is common to find some errors of this type.
|
||||||
|
* Make sure that:
|
||||||
|
* The commit message is formatted appropriately (check `Git Commit Messages`_ for more information on how you should write a git commit message.)
|
||||||
|
* The coding style matches guidelines given in HACKING.rst
|
||||||
|
* The patch is not too big (you might need to split some patches to improve cohesion and/or reduce size).
|
||||||
|
* The patch does what the commit message promises
|
||||||
|
* Unit and functional tests are included and/or updated
|
||||||
|
* If during the inspection you see a specific line you would like to bring up to discussion in the final review, leave feedback as an inline comment in Gerrit. This will make the review process easier. You can also see the `prefixes you can use`_ for Zaqar inline comments.
|
||||||
|
* Keep in mind the `Zaqar's Reviewer Guide`_ and be respectful when leaving feedback.
|
||||||
|
* Hit the "Review" button in the web UI to publish your comments and assign a score.
|
||||||
|
* Things to consider when leaving a score:
|
||||||
|
* You can score with a -1 if you think that there are things to fix. We have to be careful to not stall the cycle just because a few nits, so downvoting also depends on the current stage of the development cycle and the severity of the flaw you see.
|
||||||
|
* You can score with a 0 if you are the author of the fix and you want to respond to the reviewers comments, or if you are a reviewer and you want to point out some reminder for future developing (e.g. the deadline is the next day and the fix needs to be merged, but you want something to be improved).
|
||||||
|
* You can score with +1 if the fix works and you think that the code looks good, upvoting is your choice.
|
||||||
|
* Remember to leave any comment that you think is important in the comment form. When you are done, click "Publish Comments".
|
||||||
|
|
||||||
|
For more details on how to do a review, check out the Review section in the GerritWorkflow wiki.
|
||||||
|
|
||||||
|
.. _`Open Zaqar fixes`: https://review.openstack.org/#/q/status:open+zaqar,n,z
|
||||||
|
.. _`Zaqar's Reviewer Guide`: https://wiki.openstack.org/wiki/Zaqar/Reviewer_guide
|
||||||
|
.. _`prefixes you can use`: https://wiki.openstack.org/wiki/Zaqar/Reviewer_guide#Use_Prefixes
|
||||||
|
.. _`Git Commit Messages`: https://wiki.openstack.org/wiki/GitCommitMessages
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
BIN
doc/source/images/zaqar_review_id.png
Normal file
BIN
doc/source/images/zaqar_review_id.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 76 KiB |
@ -88,6 +88,16 @@ Setting up a development environment
|
|||||||
|
|
||||||
development-environment
|
development-environment
|
||||||
|
|
||||||
|
Your first patch and your first review
|
||||||
|
======================================
|
||||||
|
|
||||||
|
.. toctree::
|
||||||
|
:maxdepth: 1
|
||||||
|
|
||||||
|
first_patch
|
||||||
|
first_review
|
||||||
|
|
||||||
|
|
||||||
Running and writing tests
|
Running and writing tests
|
||||||
=========================
|
=========================
|
||||||
|
|
||||||
@ -107,7 +117,7 @@ Other resources
|
|||||||
gerrit
|
gerrit
|
||||||
jenkins
|
jenkins
|
||||||
|
|
||||||
Modules Reference
|
Modules reference
|
||||||
=================
|
=================
|
||||||
|
|
||||||
.. toctree::
|
.. toctree::
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user