Photo CC licensed by oedipusphinx
I've been working for canonical for about four and a half months now. One of the things that I've been really impressed with is the workflow for development and the process for how new features end up in trunk.
The workflow is as follows:
Our primary development focus is trunk as this is what's deployed on a weekly basis. So the first step is to pull the local copy of trunk to ensure it's up to date. Next, the developer makes a branch of trunk for the feature/bugfix.
cd trunk bzr pull cd ../ bzr branch trunk branches/my-awesome-feature
The feature is developed with TDD (tests first, followed by code to make the tests pass). Once the feature or bugfix is finished the branch is pushed up to launchpad under the project.
# hack hack hack # Ensure tests pass bzr push lp:~<USER>/<PROJECT>/<BRANCHNAME>
A nice feature of bzr that I'd not used prior to working at canonical is the way you can automatically set push targets for branches based on the directory that contains them. So for example we have a config in
~/.bazaar/locations.conf that looks something like this (obviously substituting the variables in angled brackets for your username and project etc):
[/path/to/branches/] push_location = bzr+ssh://<USERNAME>@bazaar.launchpad.net/~<USERNAME>/<PROJECT> push_location:policy = appendpath
So if I have a new branch at
This will automatically set the push location to:
Meaning all I have to do is use
bzr push to send my branch into launchpad.
Note: At any time you can check the push location with
The next stage is for the developer to make a "merge proposal" via the launchpad interface. This puts the merge request in a "to be reviewed queue" which provided a merge proposal page with a diff of the changes from trunk (the target branch is customisable). The rule is that every branch has to be approved by a minimum of two devs. Reviewers can make recommendations or assert that something needs to be fixed and the developer can then push subsequent changes to the same branch and those changes are made visible.
The focus of reviews are on coding style, standards e.g. adherence to PEP 8, and reviewers will run the test suite and run pylint to check for any glaring violations. Merging with trunk is also carried out to check that no conflicts occur.
Once the code has been successfully reviewed code is submitted via PQM (Patch Queue Manager) which then runs the test suite and only merges the code with trunk if the tests pass. We're still using PQM but most projects have already begun the process to move to tarmac which is tightly integrated to launchpad.
I like the rigorous review process. By having code reviews a mandatory part of the process it becomes just part of a routine, everyone is used to it and so are happy to receive and give criticism. Reviews are definitely seen as a route to better code rather than making people feel small which is exactly how it should be.
The process might sound quite lengthy but because of the ease of branching in bzr (like any other DVCS) the process is very quick and easy and branches are continually landed in trunk every day. It's actively encouraged that branches are kept around 500 lines of diff where possible, to make it easier for code to get reviewed. Branches aren't worked on for weeks on end, they are developed, reviewed and merged within what's usually at most a couple of days.
One of the things we're currently looking at is using hudson to automate the running of the test-suite for merge-proposals. This would mean that hudson could run the tests as soon as a merge proposal is made and provide an "approval" or "needs fixing" status based on the results of running the test suite. The net effect would be that we'd then require 3 approvals rather than 2.
Obviously our workflow revolves around the use of launchpad, but there's plenty of ways to adopt a similar workflow without requiring the use of launchpad. For example on personal projects I've adopted pretty much the same process except branches are pushed to remote server instead of launchpad and rietveld has been used for reviews in place of the merge proposal feature. Rietveld is fairly vcs-agnostic however, if you're using git as your vcs then something like gerrit might be worth looking at. (gerrit was originally a fork of rietveld but has more recently been completely re-written using java instead of python). Also review-board is another code review tool that works for all popular VCSs - review-board is django-based and is therefore relatively easy to set-up.
- Use a DVCS (e.g: git, mecurial, bzr) so branching is easy. I remember at Yahoo when CVS was the VCS everyone had to use, branching was explicitly avoided as much as possible because merging was seen as such a pain. Worst case, if you're stuck with something like svn be comfortable with merging so branching is not a hurdle.
- Branch as often as necessary.
- Review everything and make it a part of the code routine.
- Merge as early as possible.
- Keep Trunk pristine.