Canonical Voices

Posts tagged with 'code review'

Daniel Holbach

I talked many times about getting involved with developing Ubuntu and how it can seem daunting and that there’s much to learn. When I talked to contributors who had reached the critical point where they understood what they can do, who they can talk to and how the processes roughly work, most of them said that three things helped them to get to the point:

Code review

Today I want to talk about code reviews. It’s probably the most straight-forward way to learn by osmosis: you easily pick up conventions, distinctions which are made and which processes to follow.

Everybody has to go through code reviews, no matter which team they are in, which company they work for or when they joined the project. Up until a point they get their developer application approved and get upload rights.

This is the reason why code reviews in Ubuntu are so important and why we should constantly strive for timely replies and decisions on review requests.

Sponsoring Queue

The sponsoring queue is reviewed by developers with upload rights. Sometimes it’s very easy to approve a request and upload the package, sometimes it takes a bit longer, especially when you have a comment ping-pong between the reviewer and the patch author.

We came up with a number of points in our documentation which should help keeping the queue manageable:

For Bugs fixing small details, you could do the following:

  1. Ask the contributor to forward the patch upstream.
  2. Open an empty upstream bug task.
  3. Mark the Ubuntu task as ‘Fix Committed’.
  4. Unsubscribe ubuntu-sponsors, or mark the merge proposal status as “Work in Progress”. (Be sure to tell the contributor to reverse the process.)

This will get the review item off the list for the time being and once we can import the code from upstream, it will get fully closed.

We also get requests which are not suitable for the current release period. In this case you could:

  1. Let the contributor know that the patch is not suitable for the current release period.
  2. Unsubscribe ubuntu-sponsors, or mark the merge proposal status as “Work in Progress”. (Be sure to tell the contributor to reverse the process.)
  3. Subscribe yourself to the bug report.
  4. Milestone the bug to ‘later’.
  5. Visit https://bugs.launchpad.net/people/+me/+bugs/?field.milestone%3Alist=196 once the new release opens and upload the fix.

This are just some points which help to keep things on the queue relevant.

Patch Pilots

From the Bazaar team we borrowed the scheme of “patch pilots”. Here’s how they explain how it works: “The word pilot is in the sense of a maritime pilot: we help patches come through congested waters safely in to harbour. The main thing to watch is the bzr active reviews page in Launchpad. When you’re piloting, put some concentrated effort into helping people have a good and satisfying experience contributing to Bazaar. Just how you do that is up to you.

Instead of trying to review each and every bit in the queue – sometimes there are packages you know less about and where you can’t make a decision for example – you try to help nudge the patch along. You help to talk to upstream about it, try to find somebody who can make a decision, etc.

Canonical engineers with upload rights who work on Ubuntu are expected to spend an hour per week on the Ubuntu sponsoring queue, so everybody’s on the hook for having a piloting shift 4h every four weeks. This usually works much better, as you have an extended period of time where you do nothing else. Current patch pilots can be seen in the #ubuntu-devel channel topic.

Up until now I mostly noticed Canonical engineers who did piloted. If you have upload rights and are interested, let me know and I can add you to a preliminary schedule, so you get a reminder mail and you can try it out and see if you like it.

Please all help making this work even better. :-)

Read more
Daniel Holbach

Congratulations everyone, we got a fantastic LTS release out, some of the reactions of our users you can see here. Well done! :)

At the same time understandable, but also worrying is the look at our sponsoring page:

Silently with all the release freezes in place, the number of open sponsorship items has crept up to ~70 again.

If you are an Ubuntu developer or can help out with reviewing patches, please head to the sponsoring queue and help out. Many contributors and Ubuntu users are going to appreciate it. (Docs can be found here.)

Read more
Daniel Holbach

New contributors who don’t have upload rights to Ubuntu yet get their code reviewed and their packages uploaded by Ubuntu developers. This process is called “sponsoring” and our current process has been in place since pretty much forever. It has even gotten easier over time, so new branches or patches show up on our review queue.

Two years ago when we were struggling with getting code reviewed, we put in place “patch pilots”, a great concept we borrowed from the Bazaar team. We set up a monthly schedule and Canonical provided 4 hours per month per engineer with upload rights to make sure code gets reviewed. This has helped a lot.

Getting closer to the 12.04 release, it looks like we need to put some extra effort in and need some help.

Sponsoring Stats

That’s right we have been hovering around 50 for a while now, dealt with many incoming new requests, but still we don’t get down to 0. If you can review code, please help out.

We all are interested in getting new developers on board. This only works if we review each other’s work, gain each other’s trust and give each other advice.

The Sponsorship queue is where a lot of exchange about this happens and where knowledge is passed on. Help out by reviewing today and help grow our community this way.

This is one of the most valuable contributions to Ubuntu! This matters to all of us.

If you want to see at once glance how we are doing and who’s all helping out, head over to our one glance sponsoring page. (Patches to make it look more Ubuntu-y are very welcome!)

Check the instructions for code review (with lots of tips and tricks) and get your name on the page as well!

Read more