Skip to content

Developer's Guide

Jump to bottom Edit New page
Alex Bikfalvi edited this page Oct 17, 2016 · 6 revisions

Setting up your Gerrit Account

All patches to MidoNet are submitted to Gerrit, an open source, web-based code review system that enables a more centralized usage of Git. The Gerrit code review system for MidoNet is publicly hosted on GerritHub.

Sign into GerritHub using your !GitHub account and set up your ssh-key in Settings->SSH public keys.

Setting up your Development Machine

Register your name and email to git:

 $ git config --global user.name "FirstName LastName"
 $ git config --global user.email "your_email@youremail.com"

Install git-review:

Mac

 $ sudo pip install git-review

Ubuntu

 $ sudo apt-get install git-review

Red Hat

 $ yum install git-review

Getting the Source Code

If you set up your development environment with Midostack, there is no need to download the source since Midostack did that for you (typically under /opt/stack/midonet).

Download the source:

MidoNet

git clone git@github.com:midonet/midonet.git

Neutron Plugin

git clone https://github.com/openstack/networking-midonet

Midostack

git clone git@github.com:midonet/midostack.git

MidoNet Docs

git clone git@github.com:midonet/midonet-docs.git

Branching Model

Developers should only submit patches against the master branch for all projects. If a bug fix needs to be backported, it will be addressed in the Bugs meeting.

The master branch is where the development for the next release happens. Once released, a stable branch is cut and maintained until the support period ends.

The branching model of all the projects are explained in [Release Workflow|Release-Workflow].

Setting Gerrit Remote

Set the remote URL of gerrit by running the following command:

 $ git-review -s

The command above should add 'gerrit' remote pointing to the appropriate project in GerritHub.

Development Workflow

Make sure you have the last upstream changes:

$ git pull origin <source_branch>

Create a topic branch to do your work:

$ git checkout -b <branch_name>

Commit your changes after making sure that the unit tests pass:

$ git commit -a
$ git review

If the current branch is outdated Gerrit will warn you. Gerrit allows us to submit one review request per commit, if you have more than one commit, Gerrit will create one review request for each of them. Jenkins will run the tests as soon as you submit your review request. The result will be a -1/+1 Verified on the review request page.

Important: if you have to update your topic branch please try to avoid merge commits and use:

$ git pull --rebase origin <source_branch>

If your commit is rejected you can roll back to the last successfully merged commit on the branch. You can check its id by issuing:

$ git log

Next, run this command:

git reset --hard <last_merged_commit_id>

Submitting a Draft

To submit a review for a draft (the code is not ready to be merged but you want comments):

$ git review -D

Updating a Change

To upload your change after addressing review comments:

$ git commit -a --amend
$ git review <target_branch>

Git Commit

Message

For the commit message, follow the rules below:

  • The first line should be a brief summary of the patch, limited to 50 characters, preferably identifying the targeted component.

  • Add a blank line after the first line.

  • Provide a detailed description of the change. Each line should not exceed 72 characters.

  • Insert a blank line after the detailed description.

  • If the patch relates to a JIRA issue, add a line with 'Ref: JIRA_ISSUE_ID'.

  • If the patch is implementing a blueprint, add a line with 'Implements: BLUEPRINT_NAME'.

  • Add a Gerrit 'Change-id' line.

  • Add a Git 'Signed-off-by' line (using git commit -s or git commit --sign-off).

  • Example:

    Agent: Adds example commit message

This patch adds an example commit message, required by the Developer's
Guide, which doesn't really have anything to do with the agent. Note
that the lines don't exceed 72 characters.

Ref: MNA-999999
Change-Id: Idb8bfd1357bb128bc05a366c312c0caa687f12b1
Signed-off-by: Art Vandelay <art@vandelayindustries.com>

Content

For the commit itself, follow the rules below:

  • Only one fix/improvement/cleanup per patch. If you find yourlsef writing a commit message that says that the patch does this and that, there's a high chance you should be making two commits. Different commits make reviewing, bisecting and backporting easier.
  • Do not mix formatting or other cleanups with fixes and improvements on the same patch, otherwise you are introducing a lot of noise when reviewing, git blaming and backporting.

Coding Standards

Java and Scala

The MidoNet coding standards are based on Google's

https://google.github.io/styleguide/javaguide.html

We introduce the following modifications:

  • Line length is 80
  • Use 4 spaces, not tabs
  • Import order: use a separate category for midonet code

The following xml file contains the style guidelines for both scala and java:

https://github.com/midonet/midonet/blob/master/java-code-style.xml

Python

The MidoNet client and Neutron plugin coding standards are based on OpenStack's

http://docs.openstack.org/developer/hacking/

(Skip the Trademark and Licensing sections)

Code Review

Submitted code can be found in https://review.gerrithub.io/#/q/status:open. Anyone can review the submitted changes. A reviewer may leave comments and vote (+1, 0, -1) on a change.

Only the project maintainers are allowed to leave a +2 vote, and when a change receives a +2 vote, it is considered ready to be merged.

The possible code review vote values:

  • -2: "Do not submit". Any -2 blocks submit.
  • -1: "I would prefer that you didn't submit this". Does not block submit.
  • 0: No score
  • +1: "Looks good to me, but someone else must approve."
  • +2: "Looks good to me, approved

The current project maintainers are defined by groups in Gerrithub:

In networking-midonet, two +2 votes are required for a patch to be merged.

Code Review Checklist

General

  • Does the code perform its intended function, the logic is correct etc.
  • Is ALL the code easily understood?
  • Are defensive programming principles followed for all the code ?
  • Does it conform to our agreed coding standards?
  • Is the code as modular as possible?
  • Can any global variables be replaced?
  • Is there any commented out code?
  • Do loops have a set length and correct termination conditions?
  • Can any of the code be replaced with library functions?

Testing

  • Is ALL the code testable? i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks etc.
  • Do comprehensive tests with the right code coverage exist ?
  • Do unit tests actually test that the code is performing the intended functionality?
  • Are arrays checked for ‘out-of-bound’ errors?
  • Could any test code be replaced with the use of an existing API?

Out-of-the-box thinking

  • Don't follow this list. Follow a use-case based approach or consider any other specific areas you may be interested to probe (performance, security, etc)

Code Testing

In order to test your code, you can use Midostack or MmmUsage

Developer documentation in the MidoNet repository

Add a custom footer
Clone this wiki locally