Parasite

parasite-smiling

One thing we’ve run into at VMware is that developing GTK+ applications can be hard. Especially when creating and editing UI elements from code (as opposed to Glade), it’s sometimes challenging to debug problems. If a button isn’t showing up the right size, or something is aligned incorrectly, it could be any number of problems.

In order to address this, we’ve started development on a new debugging tool, which we’re calling Parasite. The name comes from the fact that it runs in-process with another application (specifically, as a GTK+ module) and infests it with introspection capabilities.

There’s a lot that can be said about what it does, but the best place to start is probably a screencast.

parasite-introThere’s a lot more information, some screenshots, and instructions for getting the source at http://chipx86.github.com/gtkparasite/

Tags: , ,

CodeCollaborator “Competitive Analysis”

Now, I’ve never really felt like ReviewBoard was competing with Smart Bear Software’s Code Collaborator. I’m sure they see it that way, but it doesn’t have any bearing on the way we’ve run the project. We’re not concerned about feature checklists or keeping parity; we’ve been trying to make Review Board be the best tool we can for us, and all of our contributors have been doing the same. Comparisons to other tools rarely factor into the equation, since we don’t have a lot invested in whether people choose us or another. The way I view it, any sort of code review tool is a step in the right direction.

That said, this competitive analysis published on their web site had me giggling. The sheer amount of FUD and outright lies is staggering. My favorite bit is how Review Board doesn’t support pre-commit reviews for Perforce (which was the first thing it supported).

Tags:

Single-header includes in GTK+

I keep seeing people talking about moving towards single-header includes with GTK+. I’m not sure why this is suddenly a big deal without much discussion about whether it’s actually a good thing, but I do want to bring up one concern.

<gtk.h> includes a heck of a lot of other files.

Here’s a test, building the VMware Workstation UI with and without single-header includes (on my 8-core beast):

Status quo:
real    4m34.861s
user    17m46.059s
sys     1m44.091s

Single-header include:
real    5m16.210s
user    19m56.175s
sys     1m55.627s

The actual percentage time per file is quite a bit higher than this shows; I’m estimating that at least 60% of the “real” time is spent doing mtime checks and dependency analysis in our build system.

Perhaps there’s a good reason for single-header includes, and it just hasn’t been communicated. Meanwhile, this does have a noticible impact for developers on large codebases.

On the other hand, maybe we can precompile gtk.h and everything that comes from that?

Code Review

A lot of the initial responses we get about Review Board go something like this:

Review Board looks cool, but it’s designed for pre-commit reviews. I don’t like the idea of needing review before committing, for a number of reasons. Can my use case be supported?

The short answer to this question is “Yes, it’s supported, but our UI isn’t optimized for that case.” In particular, it requires using the post-review command line tool to create the review request instead of having a nice web UI for selecting a revision to review, and it’s almost completely undocumented.

While we’re interested in generalizing Review Board into a tool which supports a variety of workflows, I’m not writing this today to explain how to make it work. I’m writing this to explain why all of you who think this are wrong.

Before I finish my attempts to offend everyone, some people should leave the room:

If you’re working in an industry such as telecom or aerospace, and you have a very intensive code review process to ensure perfection, this probably doesn’t apply. I appreciate your hard work ensuring that the airplane I’m flying in won’t fall out of the sky, and that I can call 9-1-1 when needed.

Next, if you’re not using version control, stand up, hang your head in shame, and go set up a server. SVN, Git, Mercurial, it doesn’t really matter.  Visual SourceSafe is maybe a bad choice, though. Seriously, version control is number-freaking-one on the famous Joel Test. Once you’re up and running with that (get a bug tracker too, while you’re at it) you can start thinking about code review.

Okay, now we’re down to just those enlightened, version-control-using hackers (ahem, software engineers) working on the sort of normal software that we do. I promised to offend the rest of you. Here we go.

It’s often kind of hard to define code review, since so many people do so many different things. It covers the spectrum from “occasionally glancing at a diff when a commit message sounds dubious” to those autopilot engineers I already told to leave. In the absence of any sort of authoritative definition, I’ll go with Wikipedia’s:

Code review is systematic examination (often as peer review) of computer source code intended to find and fix mistakes overlooked in the initial development phase, improving both the overall quality of software and the developers’ skills.

Why should I care?

That’s probably as good a definition as any, and I almost fell asleep before I finished reading it. Yawn. What the heck is this good for, anyways?

The big, super important reason anyone does code review is finding mistakes.  We all make mistakes, sometimes. Even those superhuman hackers who seem to write 10x as much code as everyone else where every line looks perfect. It’s really easy to stop really looking at code after having read through it a few times. Ever spent 10 minutes trying to get something to compile before noticing that you missed a semicolon? A child could look at that code and tell you what’s wrong in seconds. It’s not that you’re dumb (well, maybe), it’s probably that you stopped reading and started remembering.

I remember a paper I wrote in college. I think it was about medieval Japan. My mother offered to read through it for me, and I wasn’t about to refuse some free editing, so I sent it to her. In one sentence, I had two “the”s. That’s right. It said something like “In the the Heian court…”. I must have read that paragraph twenty times before I sent it, and I had just stopped paying attention to what was on the page. I said that even superhuman hackers make mistakes, and that means even me.

Finding mistakes is pretty obviously a good thing, I think. After several decades of using software, people have pretty low expectations for quality. You wouldn’t believe how often I tell people where I work and they start talking my ear off about how our software never crashes, and how it’s such a different experience from, well, almost everything else they have to use. It turns out that writing software is really hard. Shocking, I know.

The next big reason for code review is training. Most large software projects have some underlying architecture and design principles which can take some time to learn. One example that immediately springs to mind is the way we use signals and slots in our UI code at VMware; this sort of asynchronous design can take a while for people to get the hang of, and providing suggestions during code review is a great way to get people up to speed in the context of doing real work.

Code review also helps us improve our truck factor. This is my term for “if you were to get hit by a truck tomorrow, how many people would understand the code you’ve written?” If it’s been peer reviewed, that number will be at least one. Gas prices may be driving down SUV sales, but there’s still a lot of trucks. Be careful out there on the roads, guys.

And last but not least, it’s so we can learn from each other. I’ve lost track of the number of times I’ve seen something I didn’t understand in someone’s code and ended up learning a new trick. Everyone likes new tricks, right?

How to ruin everything

So, overall, code review seems like a pretty good thing. We can improve the quality of our software, train new employees/contributors, and learn stuff all at the same time. Now lets look at how people do it wrong.

The first extreme I’ve mentioned is where everyone gets together and pores over some code to find every problem with it. If you’re doing this and your software doesn’t have lives on the line, stop it. Perfect code is a nice ideal, but “sometimes it is not worth fixing a bug.” There’s no way that an entire codebase can be scrutinized in this detail and still live in the fast-paced world of software.

On the other extreme, the easiest way to do it wrong is just not to do it. Sure, you may get an extra few hours a week to surf pornography and make faces at the dude in the adjacent cubicle, but it’s not gonna help the quality of your software.

So, assuming that you’re somewhere in the middle, and you’re doing code reviews without interrupting your developers’ lives too much with hours of meetings, you’ve probably fallen into one of two camps: pre-commit and post-commit. There are some pros and cons for each, but to me, the choice is obvious. Here’s why.

In a post-commit environment, a developer makes some change to the code and submits it to version control. At this point, usually in an automated fashion, that change is sent out to the other developers. This almost always takes one of three forms:

  1. The commit message and a list of files is sent to a mailing list (lame).
  2. The commit message and a diff is sent to a mailing list (slightly less lame).
  3. The change is automatically made available to a tool like Review Board or Trac (okay, I guess)

At this point, those other developers can look at the code and see if there’s anything they disagree with. The big, major, 600-pound hairy smelly gorilla of a problem with this is that /they don’t/. For example, one of the people who checked out Review Board and asked about post-commit reviews wrote this:

Sure, some bugs can (and do) creep in because they are missed in larger commits where we don’t read the entire email, but I’m not worried because QA should catch those…

It’s exactly the larger commits that need the most review! Any developer writing a three line trivial bugfix patch can convince themselves fairly easily that it’s not going to completely fuck shit up (that’s a technical term). It’s a bit harder when the patch is thousands of lines and implementing complex functionality.

Also note that unless you’re working on a private branch (which is fairly rare), committing that big patch before review has a good chance of breaking stuff. Raise your hand if you’ve done an svn up (p4 sync, git pull, whatever) and the code you pulled down didn’t build. Or crashed in a super obvious place as soon as you tried to run it. Everyone? Yeah.

The reason for this is because most people are amateurs, and those who aren’t sometimes get sloppy. Shortly before I started working at VMware, they split the main perforce depot into three separate main branches. They couldn’t componentize the code, so they decided to componentize the developers into branches based on the organization they worked on, in some meager hope of making it so people could actually get work done instead of spending all day hunting for a changeset that wasn’t broken.

That’s right, with the most amazing group of hackers I’ve ever had the pleasure of working with, they couldn’t keep *main* building cleanly once there were more than a few hundred people working on the code. I’ve heard similar stories from many other engineering organizations that I deeply respect. If they can’t do it, what makes you think that you can?

Realistically, you can’t. Noone can. That’s why we have to take steps to try to prevent it. Steps like branching. And, surprise surprise, code review. And tools. Because if the build is broken, it’s not just you and your QA contacts whose time is wasted, it’s every single engineer you’ve got.

Why would anyone want to do it wrong?

So why do people do this? I’ve seen two common reasons.

REASON NUMBER ONE! “Because that’s how people are used to doing things.” This is a powerful reason, and I won’t deny that it’s very hard to get people to change their ways. However powerful inertia is, it doesn’t mean it’s right. It took a lot of effort to get doctors to start washing their hands, but I’m pretty happy that it changed.

Number two, and this is the big stupid one. “Because we’re a fast-paced development shop, and it would slow us down.” I’ve heard this from several people. It’s particularly hilarious to me, because they talk as if slowing down is a bad thing. How many MP3 players hit the market before the iPod?  How many search engines came and went before Google? Being first to market is great, but being best to market is better.

And finally, a silver bullet

So how do we solve this big morass? Simple. Review code before it’s committed. If you’ve got branching that doesn’t hurt, commit to a branch and review it there, but before it affects anyone other than the original developers. Slow down, pay attention to what you and your teammates are doing, and you may find that you’re not quite so good at what you were doing as you thought. And once you’ve realized that, it’s hard not to want to take every possible step to improve.

Finally, before you comment, keep this in mind: I intentionally wrote this in a confrontational tone for my own amusement, and to encourage people to talk. I’ve been doing this software thing for a while, but I don’t have anywhere near all the answers. Think I’m wrong about all of this? Let me know!

Tags: , , ,

REAPER

ProTools is jaw-droppingly awesome.  Audacity is minimal but sometimes adequate.  Ardour is just frustrating.  I wish Jokosher the best of luck, but right now it’s a toy.

I just read about REAPER today, and downloaded it.  All 3MB of it.  It’s not ProTools, but it’s pretty impressive (and doesn’t require buying an M-box).  I don’t have any projects right now, but next time I do, I’m going to give it a serious shot.

Windows only, alas.

Open Mondrian?

One of the things Christian and I looked at as we were getting Review Board off the ground was the tech talk on Mondrian. At the time, it was disappointing to us that Google had created this great tool, and kept it inside. Well, it’s still inside, but Guido has just announced a public code-review project based on the idea of Mondrian.

I played around with the demo server he’s got running, and it’s still pretty rough — a lot like Review Board was back when we first did our first public announcement. Since that time, we’ve had an amazing ride, with lots of contributions and the building of a fairly vibrant community. I wish the yet-to-be-named Mondrian-like project similar luck; code review is one of those things that can be a major drag, and having an excellent tool makes all the difference in the world.

Just as a reminder, if you’re involved in this year’s Google Summer of Code as a mentor/student/organization, we’re offering to host a server for you.

Tags: , , ,

GUADEC

GUADEC so far has been lots of fun, as usual. The usual combination of friendly hackers and provocative ideas is very energizing, and the English ales are delicious. It’s also awesome to hear from some folks who have set up or are thinking about Review Board.

When we were discussing attending GUADEC at VMware, we tried to figure out what we could do. It seems like T-shirts are such a commodity these days that they’re no fun. We considered giving out umbrellas, which I still think would have been a great idea given the location. There was one idea that really stuck out, though.

We have a small supply of licenses to VMware Workstation 6.0 available to give away. We’re going to try to give them away either at Alex’s follow-up talk or at the party tonight. In particular, if you have something really awesome that you want to apply Workstation to, you may convince us to give you special priority :)

As usual, VMware is hiring! If you have any interest in working with a bunch of awesome hackers on a super-exciting technology (in some sweet brand-new buildings), come talk to me or Alex

Tags: ,

VMware’s Amazing New Campus

About 8 months ago, VMware broke ground on what was to be the largest construction project in Palo Alto in the last decade. Last week, the first lucky people moved into building A, and today, those of us in the Hosted UI group (who do Workstation and the like) moved into building B.

I think it’s safe to say that no expense was spared. The image below is the common area in our building. A lot more furniture is scheduled to arrive later on. There are lots more photos in my Flickr Set.

Building B Common Area

Two of the four office buildings are now filled with people. Construction on the other two is still going on, and virtually no landscaping has been done except for a giant oak tree which was transplanted into the courtyard. Sometime next year the soccer field and gymnasium will be finished.

Does this look like an awesome place to work? We’re hiring :)

Tags:

Review Board: post-release madness

Everyone seems to be loving review board. Here’s some of the crazy stuff that’s happened this last couple weeks:

Here’s what some folks using the system are saying:

Review Board is so awesome it brings tears to my eyes. We’ve been needing this so badly for so long, and now it’s here and it’s better than my wildest dreams! Thank you so much! Also, thanks for making it free!

Just so you know, the people in my team that use ReviewBoard are loving it! Thank you!

These two emails have had us glowing like proud grandmothers. :) Who says open source is a thankless job?

Tags: ,

Announcing Review Board

Christian Hammond has a great introduction to this project, so I’ll keep this relatively short.

For the past several months, the two of us have been using our spare time to work on a web-based tool for doing code review. This is traditionally a big hole in software development infrastructure, and everyone tries to fill it with things like mailing lists or bug trackers. We got tired of dealing with problems such as being unable to easily find out which patches needed review, or losing someone’s comments deep within our inbox. These problems aren’t unique to commercial software either; one of the biggest gripes within the GNOME project is that it’s hard to keep track of everything needing review.

It’s been a heck of a ride over the last few weeks. We’ve rolled it out at VMware to two groups so far (Hosted UI, where we work, and Lab Manager, who were similarly fed up with email and decided they wanted to start using this immediately after seeing a demo). We finally decided, with a lot of the kinks worked out, that we’re ready to announce it to the world.

Review Board is hosted on Google Code, and released under an MIT/X11 style license. There are lots of goodies in the source (such as an NIS backend for django’s authentication system) that we hope are useful to people, even if they don’t use Review Board itself.

And, since new software can’t be announced without screenshots, here are a couple of the screenshot support within reviewboard (for that double-screenshot taste). There are lots more screens on Christians’s announcement.

request-with-screenshots-t.png

screenshot-comment-t.png

Tags: , ,