Archive for the ‘git’ tag

A simple code review script for Git


This January, Ksplice swelled from 8 people to 20 people. You can imagine what that did to the office—it’s a good thing that Tim and Jeff have been practicing the art of rearranging a space to fit more people than ever thought possible since their days in the SIPB office. Fortunately, because we have at our disposal a computer-systems training and recruitment machine of awesome effectiveness, our interns defied Fred Brooks and produced a great deal of useful code.

The problem: how to keep track of all that new code and get it all reviewed smoothly? Our ad hoc practices relying on one-on-one exchanges clearly were not going to scale. The solution: on the first day the new interns showed up, I took a couple of hours and threw together a script to request code reviews. The key design considerations were

  • Public visibility. The script sends mail, and CCs a list going to the whole team.
  • Non-diffusion of responsibility. The user must identify someone to be the reviewer, and the request is addressed to them.
  • Git friendliness. Being a kernel shop, we use Git for everything, so the script assumes a basic Git workflow: you make some commits, and then you request a whole series of commits to be reviewed at once.

We looked at some existing code review tools like Gerrit and Rietveld, but we weren’t happy with any of them because the ones we found don’t work on branches—they work on individual commits—and we have drunk too deeply of Git to be satisfied working that way.

On the other hand, being the product of a few hours’ work, there’s several things that could be made better. The interaction with the user could be better to prevent mis-sends. The script could do better at detecting what the repository in question is called, it could take advantage of commit messages for better subject lines, and it could try to give the reviewer a command line that will show the commits in question. (Until then: it’s usually git log -p --reverse origin/master..origin/branchname.) Someday we may also want a system that tracks what commits have been reviewed by whom and blocks commits from going in without review; that will be a bigger project.

Apparently we did something right with this script, because I heard a couple of people say they’d like to use it outside of Ksplice. So the other day we decided we were happy to release it as free software. As of last night you can get it from Github—enjoy.

Let me know if you use it, and patches welcome, of course.

Written by Greg Price

February 1st, 2010 at 3:56 am

Posted in Uncategorized

Tagged with , , ,

Read-Write Software

leave a comment

My favorite moments with free software are when I get annoyed with some manual task that a tool leaves me to do for myself, and then invent a feature that the tool should have to handle the task for me.

With any software free or proprietary, if I’m lucky the tool might have a configuration system powerful enough to let me effectively add the feature from the outside. But with free software, I don’t need the authors to have anticipated my needs—I can reach into the guts of the software itself and change it to work the way I want. If it’s a friendly codebase or if I’ve hacked on it before, I may be able to add my change in a few minutes. And hey presto: software that does exactly what I wanted. It’s a lot more fun than praying to the vendor and waiting a few years, and it’s faster and more reliable too.

So it went with Git one night last October. I was repeatedly revising a branch with git rebase -i. A couple of points along the branch were marked as branches of their own, so every time I changed something I would have to either

  • rebase the full branch, then do a dance with checkout and reset to update the sub-branches, carefully typing the correct new commit IDs;
  • rebase the full branch, then muck with update-ref with the same care about getting commit IDs right; or
  • rebase the first sub-branch, then use rebase --onto to move the next sub-branch on top of it, then rebase --onto again for the main branch

What I really wanted to do was just

  • rebase the full branch, and tell the sub-branches to come along for
    the ride.

Fortunately I’d worked on the code for Git’s interactive rebase before—at Ksplice we push Git to its limits in six different directions, and rebase -i we push beyond the limits of stock Git—so I knew where to find the moving parts that could do what I needed. Four minutes after having the idea, I was happily using the new feature.

If you want the feature too, it’s up on my Git git repo. Or you can wait until I get it upstream. Why haven’t I done that already? That’s another old story about software. My 4-minute, 4-line patch turned into 29 lines with documentation and with proper error handling, then 147 lines to make the feature easy to invoke, and then 231 lines with test cases. So I just finished all that work today. Maybe you’ll see the feature in Git 1.7.1 this spring.

Written by Greg Price

January 25th, 2010 at 2:30 am

Posted in Uncategorized

Tagged with , , ,