Mergimus: Making Patch And Branch Review Easier In Ubuntu

Recently I blogged about how we are keen to improve patch review in Ubuntu in the 10.10 cycle. Today we are in a position where we have a large collection of fantastic contributions that need reviewing, even if that review feedback is that the patch no longer applies and needs work.

I believe part of the problem here is that reviewing patches is just too hard. At the Ubuntu Global Jam on Friday I was talking this through with a few people and I started drilling on the idea of a desktop tool that improves viability on patch contributions and automates much of the work involved. I believe this tool could greatly open up the world of patch review to more people.

This morning I decided to formulate my thoughts a little better and draw up some mock-ups of a the app which I am nicknaming Mergimus. Unfortunately, I don’t have the time to commit to writing this app, so I wanted to blog the design as an opportunity to kick off some discussion and to maybe inspire some of you good folks to pick up the design and start building it. Let me explain my thinking…

The basic idea with Mergimus is that you are often interested in and have a core competency and awareness of specific projects and you want viability on the patches and merge proposals that are part of those projects. When you fire up Mergimus you would see the patch view screen:

In the left box is a treeview with a list of projects that you are interested in: each of these are source packages in Ubuntu. You can add a new project to the list by clicking the button underneath the tree view and a dialog box will pop up where you can enter a project name, and a list of matching projects will appear, and when you double click one it will add it to the main treeview.

When you click on a project, the box to the right displays a list of the patches and merge proposals available for that project. This immediately provides a TODO list of things that the user can work on, solving the first problem many prospective contributors have – “what can I work on?“. If the user wants to see all patches and merge proposals for all the projects they are interested in, they can click the All option at the top of the tree view.

When the user decides that they want to work on a given patch or merge proposal, they double click it and the view changes to the Review View:

This view has a series of key components. First, at the top of the view we see the Currently Reviewing label and a breadcrumb trail – clicking each item in the trail will open the relevant Launchpad project page in your web browser.

On the bottom-right of the view you can see a treeview with a list of changed files that are part of the patch or merge proposal. Clicking on a file will load it into the top part of the main body of the view which is a text editor. Inside the text editor a diff will be shown with the differences between the patch and the source package’s file. Each diffed file loads in a different tab.

Above the file listing tree view are three buttons:

  • Test Patch – (this will say Test Merge Proposal if a merge proposal). When you click this button, the project’s code will be downloaded and the patch/merge proposal will be automatically applied. When you have clicked this button, you can then use the embedded terminal at the bottom of the main view to go and run and test the patch. This makes applying patches incredible simple: you just click a button. If for some reason the patch or merge proposal has conflicts, a dialog box will appear providing details and possibly a button you can click which re-directs you to the bug in Launchpad to provide feedback that the patch did not apply.
  • View Bug – (if applicable) if you want to view the bug that this patch/merge proposal fixes, click this button and it loads into your web browser.
  • Leave Feedback – when you click this button you can go and provide input on the patch in Launchpad.

This is very much a first cut of the design, and is entirely open to discussion and improvement. The core idea is that we provide a TODO list of interesting patches/merge proposals to review based upon what the user is interested in, make testing them a single click, and make providing feedback a single click.

Writing this application would not be particularly complex from what I can tell. It is clearly a good candidate for a Quickly app, could use launchpad lib, and can use existing patch/diff/bzr tools. If one of you is interested in making Mergimus a reality, I recommend you just start writing some code, pop it in Launchpad and focus on a simple first cut and we can continue the discussion.

Thoughts?

  • http://breinera.wordpress.com/ Andrew Breiner

    Quick question how is the “Fix Some Bug (#123456)” and the “View Bug” button different? However this looks great. As I am done teaching in mid-May I might give this a go come then, if I have some free time that is.

  • jono

    They are basically the same: the breadcrumbs are just less visible.

  • https://launchpad.net/~flimm Flimm

    Sounds like a Google Summer of Code project!

  • http://audidude.com Christian Hergert

    This would be a pretty fantastic MonoDevelop addin.

  • http://breinera.wordpress.com/ Andrew Breiner

    Quick idea that I had and I don’t know if it is doable and I am slightly concerned that it duplicates some of ground-control, but I think it would be cool if the main page had third tab named “bugs”. Now this would list the bugs associated with the project. When the user double clicked a bug, it would bring up the second screen and allow them to work on fixing the bug. Now the three buttons could say “Text Code/Fix”, “Commit Code” and “Propose Merge” or something to that effect. There would have to be some mechanism that the user had the current code and that there was no patch against the bug. Just a thought though.

  • Markus Korn

    Sounds like a very good idea, to implement the patch part a method to get a collection of all patches for a project needs to be exposed in the launchpad API first.

  • http://launchpad.net/~dobey Rodney Dawes

    Is this meant to be a general solution for all projects on lp, or is it primarily designed to deal with patches in ubuntu packages?

  • jono

    We already expose this in Launchpad with the patch view. E.g: https://edge.launchpad.net/ubuntu/+source/samba/+patches – I am not sure if this exposed in the LP API though.

  • jono

    To be frank, my primary concern is Ubuntu, but there is no reason why it could not be extended to serve upstream LP projects too.

  • jono

    My hunch is that would take Mergimus out of scope, and we have tools such as Bughugger and Ground Control for this. :-)

  • http://sourcefrog.net/ Martin

    There’s some example API code to get and update merge proposals in Hydrazine, here: http://bazaar.launchpad.net/~hydrazine-core/hydrazine/trunk/annotate/head%3A/feed-pqm

    It would be great if this went into something with a UI like Jono proposes.

  • Stephen Gentle

    There’s a absolutely massive usability problem in that screenshot – the maximise, minimise and close buttons in the window border.

    I have actually stopped using Mutter/Gnome Shell and gone back to the normal Gnome panel with Compiz and Emerald window manager in the new Ubuntu alpha since that was changed…

    Anyone who has ever used Mac OS X is guaranteed to find this layout weird and confusing… I really think you should either put them in the right order (close is always at the far left or far right edge of a window) or put them back where they were.

  • http://www.nixternal.com nixternal

    “I believe part of the problem here is that reviewing patches is just too hard.”

    I don’t think that the actual reviewing is to hard. People who should be reviewing patches should be able to look at the patch and understand it, at least have an idea of the code base the patch is going to be used for. I think the process of finding, getting, or being notified of patches could be streamlined or made better. As it stands, I get over 1,000 emails a day, and as a volunteer that is inbox overload. So if I get a patch email, there is a damn good chance I miss it. Even if I had an app that could locate all of the patches on LP, there is a good chance I would forget about that as well. Hell, I tend to forget to clean out my TODO list like I should on a weekly basis.

    I don’t think throwing a GUI app at this fixes the problem, but possibly adds yet another layer to the underlying problem.

  • http://rbtcollins.wordpress.com/ Robert Collins

    apt-get install lptools review-list

    This is heading in the direction you describe, and we’d love more contributors.

  • http://www.nixternal.com nixternal

    I knew I forgot something in my reply. Yes, what Robert said, and if the pay is good, I just might contribute there :)

  • jono

    Please don’t disrupt the thread with this: there are places you can go and talk about this…

  • jono

    I think Mergimus would solve exactly the problems you outline: you want to have visibility on things that are interesting to you to work on but don’t want all of the bug mail.

    As I said in my post, Mergimus only eases the visibility of finding patches and applying them: it smooths the mechanics of how we review patches, but fundamentally the act of reviewing a patch itself still requires suitable skills of the code and the patch.

  • http://www.nixternal.com nixternal

    All Mergimus would be, is LP on my desktop. If I can’t remember to check patches every day or once in a while, why am I going to remember Mergimus? Of course this would be a different story if my employer made me do this, but in this case, as a volunteer and a fairly busy one, I am spending time on other things. I have “Review Patches” on my todo list, but I rarely get to it. Patch review is a necessity honestly, and we need more people doing it. Do you think that Mergimus will get more people who are qualified to do patch review, reviewing?

    If anything, we need something to ping us on our phones, IRC, airplanes, or even one of those blimps you see on TV with advertising. Just send one over my house everyday and say “HEY! PATCH REVIEW DANGI!” :)

  • https://launchpad.net/~mpontillo mpontillo

    Thanks for the write-up. I think this is a great idea, but then, I’m somewhat to blame since I was agitating this discussion at the jam. ;)

    I have been dabbling in Ubuntu packaging, patching and such for awhile, and one problem I see is that the process is too complex for your average “drive by contributor” (I hate that phase but it’s somewhat accurate) like myself, who has a few minutes here and there to contribute. There is a lot to learn and remember. What I want is a tool that abstracts away the details and lets me focus on the problem I’m solving, and maybe allows me to easily keep tabs on the package in case I introduce a regression. (and maybe even makes it easier to prevent regressions in the first place!)

    I can imagine a tool like this helping with: (1) Creation of patches (to help someone who just wants to fix a simple bug do it properly) (2) Patch testing and review (someone validating (1)) (3) Diffing between Debian and Ubuntu sources / patches, or between Debian/Ubuntu and the upstream source (someone doing general package maintenance) (4) Ensuring patches comply with the DEP-3 patch tagging guidelines (5) Keeping changelog entries honest (if you know from Launchpad what exact patches/bugs you’re fixing, you can make sure they’re all included)

    I know there are some very skilled maintainers who already know how to do this stuff by hand from the command line, and that’s great if you’re constantly doing this stuff, but if you’re only able to contribute here and there every couple weeks or months when you get a chance, it’s handy to have an easy-to-use application to oversee your work.

    Continuing that line of thinking, I particularly like the terminal window at the bottom of Jono’s mock-up, but probably not for the reason he put it there. Whatever the tool does, I’d want it to transparently show the command-line version of what you would do from a terminal window. That way using this program also is a tool for learning.

    Soon you start getting into more nuanced questions, like how, exactly, does one test a patch? Is it enough to use the binaries built out of the source directly? Do you have to install the package? (and I guess it really depends, but depending on what has changed you really want to install the .deb somewhere and test it) Is there a test plan somewhere? Anyway, I think a “Test” operation is something that is difficult to abstract away. I think you’d want a few operations: “Build”, “Create .deb”, “Install .deb”, maybe “Launch terminal”, where launching the terminal would allow you to do whatever you want in the source/build directory at your own risk.

    Anyway, there is probably a lot of scope creep here, but I think there are a lot of ways to make the entire process easier, so that more people can contribute.

  • jono

    I think you are missing out a few key points here:

    • Yes it may seem like LP on your desktop, but LP provides a vast array of content that can be difficult to navigate through. Mergimus would provide a way easier experience as it focuses purely on patches.
    • One thing that LP can’t do is perform the role of applying a patch and testing if it works. Mergimus does that for you: you simply select a patch, click a button and it applies and provides you with feedback.

    Simply put, if we need to review patches today, we need to download code locally and apply it, test if it works and then do something in LP. Mergimus merely provides an easier UI for the things you have to do locally on your computer.

    I think the other point is that patch review is indeed a necessity and it is something we struggle to find contributors to play with, as such I think it is open season to explore tools that can make the prospect of reviewing patches more attractive, and Mergimus is one such example.

  • http://www.nixternal.com nixternal

    Ahh, I got ya now. If it makes people want to review patches, then I am game I guess.

  • http://www.nixternal.com nixternal

    and one problem I see is that the process is too complex for your average “drive by contributor”

    @mpontillo – is it to complex, or is it just stupid? I am for the latter actually. After using a ton of other project management solutions, I will admit that patch management in LP is lacking when compared to other resources, though it’s not as bad as some. Lets face it, patch review sucks, no matter what you use for it. The last company I worked for had used Review Board, and it was tons better and much easier. KDE uses it, and as a matter of fact, it might be so easy that I am seeing more and more drive by developers (I kind of like that term).

    I would like to see some type of fix in LP before I would like to see an application that is nothing more than a band aid to the current process, though band aids aren’t always evil.

  • https://launchpad.net/~mpontillo mpontillo

    @nixternal – thanks for bringing Review Board, to my attention; looks like a nice tool.

    I agree that it would be nice to have LP improvements in this area. One of the questions I posed was, why not have this be a web-based tool, integrated into LP itself? And maybe that is the best long-term path… but the barrier to entry is higher. I would think that the requirements for adding features to LP are that they should to be scalable and rock-solid. Creating a desktop application distributes that work to the end users (developers). Maybe an application like this is a proving ground for the use model (and future process improvement).

    You mention the need for notification about pending reviews. Why not have this tool launch at startup and notify when one of your subscribed packages has a pending “action item”? (merge from upstream, patch, etc)

    As someone used to working with source control systems, branching, merging, etc, on a daily basis, I think the things that confused my the most about Debian development were all the layers. You have an upstream source, which may or may not be in a SCM tool. Then you have Debian metadata and patches on top of that [in the form of a large patch]. Then you have Ubuntu changes to that, which are a “copy” of the Debian changes that are synchronized (merged) back and forth. Once you get the Debian/Ubuntu changes applies you aren’t done, you have to deal with whatever SCM tool is managing the Debain/Ubuntu diffs, and you have to figure out how to interact with the patch system in use for that particular patch. Then once you’ve figured that out you need to make sure everything is compliant with the Debian policy and you’re correctly using the tools that help with that.

    I have a hard time imagining how LP can “fix” this. Since some amount of manual intervention may be needed, it seems to me that something more along the lines of an IDE for Ubuntu packages is needed.

    Maybe what is needed is a core python library that we could build on top of with PyGTK user interfaces, Eclipse plugins, etc. Then eventually whatever pieces that make sense could be pushed into LP?

  • http://launchpad.net/~dobey Rodney Dawes

    That’s where I was going with my question regarding distro patch review vs. upstream projects on lp.

    It’s (review-list) a simple script right now, as I wrote it quick to get something useful for our team. However, I do want to be able to completely do reviews from it without hitting the web. :)

  • http://launchpad.net/~dobey Rodney Dawes

    There is a script in lptools called lp-review-notifier which actually does almost exactly what you suggest here. It notifies you of new merge proposals to the projects you list in its config.

    However, if you’re a pretty active developer, and work on fairly active projects, notifications can get unwieldy and annoying.

    RE: python library that you can write UI on top of, we have that… it’s called launchpadlib. :)

  • http://nathguil.free.fr Guyou

    The idea behind Mergimus is really interesting.

    I’m an upstream maintainer (not an Ubuntu dev.) and occasional free developper (I do it on my spare time). So, my problem is effectivelly the problem addressed by Mergimus. Here is the scenario: « Well… I have 1 or 2 hours to offer, what can I do? »

    But my matter is not focused on Ubuntu. A contrario, I really need the full LaunchPad on my desktop: an application able to collect Ubuntu+Debian+Mandriva+*BSD bugs trackers and other patches sources like GitHub and SourceForge (the most complex are patches only sent to the mailing-list).

    Currently, I try to address this issue with RSS syndication, but it is not really easy. So, an application like Mergimus can be really really usefull.

    But I share the feeling of @nixternal: I’m not sure such an application will create passions for reviewing tasks.