Review Board

Reviewing code can suck

The open source world has given developers great tools to make their lives easier. We have editors, bug trackers, source code management tools, repository viewers. Bugzilla, for example, is a very popular bug tracker used by many open source projects and companies alike.

While the life of a developer has in many ways been improved by these tools, there’s one key area of software development that people still do the hard way: Code reviews.

You’re probably familiar with this. A contributor puts a patch up on Bugzilla, Trac, or your project’s listserv. It’s large and spans several files. Eventually you get around to it (if you haven’t lost it in your inbox yet). You open an e-mail or the Bugzilla page to respond, put the diff where you can see it, and start going through the changes, line by line, making comments as you go. It’s tedious. You have to make it clear what function you’re talking about, make references to the general area in the diff. It’s a pain for the contributor too, because they have to figure out what you’re referring to. This leaves room for error.

We’ve been working this way for years in open source projects and at VMware. Code reviews are important to us, and help to keep our code clean and our products stable. It keeps us honest prevents us from cutting corners unnecessarily. However, the process is a bit time-consuming and not at all optimal.

  1. Generate an htmldiff showing the old code and the new code side-by-side, highlighting the changes.
  2. Write an e-mail to the target reviewers/listservs explaining what changes we made, the testing that was done, links to screenshots we put up on a file server, a link to the diff, and whatever else.
  3. Wait for someone to go through the htmldiff and comment on the changes in a new e-mail.
  4. Go through the comments, try to find the lines they referred to in the diff, and fix them up.
  5. Make another diff if necessary, repeat.

David Trowbridge and I finally got tired of it. We spent too long preparing review requests and we lost too many of them in our e-mail. As our personal projects and our team grew in size, it became harder to keep track of all the open review request e-mails on the listservs. So we fixed that.

Review Board

We built a code review system called Review Board. Like most projects, it started out simple, but grew to be pretty powerful and useful quickly. It was designed to automate and simplify the process of creating review requests and actually reviewing code.

Management at VMware was excited about this from the beginning. We weren’t being asked to write it, and it was a personal project and all, but when they found out we were doing this, they fully supported us. It didn’t take long for the excitement to spread across many teams, and we had 40 people signed up and playing before we were even ready to announce our phase 1 beta.

One of the things we decided from the beginning is that this must be open source and it must work with other version control systems. For example, we use Perforce internally, but there’s no reason not to include Subversion or anything else.

So we worked and worked in our spare time. After weeks of trial runs and lots of dogfooding, we decided it was time to announce what we had so far.

Enough yapping, details already!

Details? Alright.

Review Board is an open source program licensed under the MIT license. It was designed using Python and Django. It’s compatible with Subversion and Perforce, and can be extended to support other version control systems.

Review Board has a lot of useful features…

Multiple Repositories and Projects

Review Board can generate and display diffs against multiple repositories on multiple servers, each with their own version control system. At VMware, we have a repository for each Perforce server we use, and will soon be adding a Subversion repository for our libview project.

This would be particularly useful for umbrella projects where some parts are available in one repository and others in other repositories.

Writing version control system backends is easy. They’re simple Python modules that can be referenced in the Review Board database entry for the particular backend. We currently ship Subversion and Perforce backends, as well as a subclass of the Perforce backend making use of special extensions we use at VMware.

Review Requests

Posting review requests is fairly easy. If you use Perforce, it’s especially easy. A post-review tool is provided that allows you to post a review request with nothing other than a change number as a parameter.

If you use Subversion, a little more work is required, though we’re working on a tool to automate this as well. You’ll need to generate a unified diff. Click “New Review Request,” select the repository, enter the base path (the path relative to the root of your Subversion repository where you generated the diff), and then select the diff. Click “Create Review Request.”

You’ll be taken to your review request page where you can fill in description and other information. Click “Publish” when you’re done.

Review Request

Powerful Diff Viewer

Diff Viewer
  • Inline commenting: Instead of jumping to your bug tracker or e-mail client and describing where you’re commenting on, just comment directly on the line! Click the line number or click and drag to select multiple line numbers and a comment dialog will pop up allowing you to type. When you finally publish your review, the lines in the diff you commented on will be shown inline with the rest of your review.
  • Inter-line diffs: Sometimes a small change is made to a line and it’s not readily apparent what the change was. In these cases, we highlight the regions between two lines that have changed.
  • Revisioned Diffs: Each revision of the diff is saved and can be accessed. In the future, we will make it possible to show differences between two revisions, to ease review of incremental patches.
  • Whitespace highlighting: Trailing whitespace is automatically highlighted. Trailing whitespace is a pet peeve of many developers and this helps to catch it early.
  • Collapsing/expanding of files: By default, only the changed chunks and some lines of context around them are displayed. This can be quickly changed to show the entire file, in case there’s something you want to look at or comment on elsewhere.
  • Keyboard shortcuts: Convenient shortcut keys allow quick navigation around the diff. For example, pressing “n” will jump to the next changed chunk, and pressing “p” will jump to the previous chunk. This will be further improved and documented soon.

Comment Dialog

Interactive Screenshots

  • Screenshots can be easily added to the review request page, and will show up as thumbnails.
  • Screenshots can be commented on by clicking and dragging an area on the image. A comment box will pop up, like in the diff viewer. While the clipped part of the screenshot does not yet appear on the review, it will soon.

Contextual Discussions

Review Discussions

  • Commented lines in the diff are displayed with their comments on the review page.
  • Commented areas of a screenshot will be displayed with their comments on the review page.
  • Discussion of reviews take place on the reviews themselves, making it possible to read the whole discussion of a change from top to bottom.

E-mail Support

All discussions and updates get sent automatically to the individual reviewers and listservs. This makes it easy for people to see any and all changes. This is configurable and can be disabled if it doesn’t fit in with your setup.

NIS Support

Review Board can use an NIS server as an authentication backend, making it easy to integrate into companies that use NIS. Everyone’s accounts will Just Work in Review Board without any registration required.

User Dashboard

Dashboard

All incoming and outgoing reviews can be seen on the Dashboard page. You can look at incoming reviews sent directly to you, sent to a group, or to either (all incoming reviews).

In the future, this will show more information on what’s going on with the review requests in your list. Recent discussions and updates will be shown, making it easier to keep track of things.

JSON API

Review Board has a JSON-based API for accessing nearly every aspect of the system. This makes it easy to build tools around Review Board. Our post-review tool uses this, as does much of the web UI.

Hopefully in time, developers will make use of this to better integrate with existing systems such as Eclipse, vim, emacs, or other IDEs.

Looking Forward

There’s a lot we have planned. Review Board is still just an early beta and a lot will happen.

  • Improved tools: We’ll soon be working on a single tool that understands how to post review requests to both Perforce and Subversion. It will also be able to integrate with multiple projects. For example, if three projects you’re contributing to all use Review Board, post-review will ask once for the Review Board servers and use them from then on when run in the project directories.
  • Interdiffs: We plan to include the ability to show changes between revisions of diffs, aiding in reviewing incremental changes to patches.
  • Statistics: Information will be available showing how many review requests, reviews, and comments have been made between specified time periods, complete with fancy graphs. We’re working on ideas for what can be displayed here.
  • Status Reports: This is perhaps more useful inside companies. At VMware, we’re supposed to submit weekly status reports discussing what we’ve done. Often this consists mostly of review requests made and bugs fixed. Review Board will be offering a page showing this information in several forms so that it can be easily copied and pasted into a weekly status report.
  • Integrated Help: To simplify usage for first-time users, we’d like to add help information to all pages explaining simply how to use the interface.
  • “Effective Lines” Display Many diffs are easier to review than they seem, as there could be hundreds of lines with nothing more than a function or variable name change. We would like to make this clear before even opening the diff by showing the number of effective lines changed, factoring out the simple redundant changes.

Wrapping Up, and Contributing

This was a bit long, but I hope it gave a good overview of what we’re putting together here. You can see it in action or visit the project page. If you decide to use it in your project, please let us know! Check the Wiki for install instructions.

Hopefully this will become useful to others as well. We’ll post periodic updates as this project progresses.

May I present VMware Workstation 6.0

Good news, everyone! After several months of hard work, many late nights, and thousands of caffeinated beverages, we have finally released VMware Workstation 6.0. You may have already heard this on the news, but I’m here to tell you exactly what we spent so much time on. Or a glimpse of it, anyway.

  • Windows Vista support

    We’ve added much improved support for Windows Vista. Yes, you could run it before, but not this well. We have VMware Tools support for many things in Vista now, providing for a smoother experience. A warning, though. Vista is pretty heavy on resources and may still be slower than you’re used to. Not to mention the fact that Microsoft will make you buy the really expensive version in order to legally run in a VM. Still, if you’ve been wanting to see what Vista is like, have a decent machine, and an MSDN license or legal access to a legal version of Vista, install it in a VM and give it a try.

  • USB 2.0

    We now support USB 2.0. Your fancy USB 2.0 devices should now work just fine in your VMs.

  • Headless VMs

    Starting in Workstation 6.0, your VMs are no longer tied to your UI. Previously, any time you closed Workstation, you would be asked if you wanted to power off your VMs. Now you’re given the choice of continuing to run your VM in the background. It will continue to run without a UI.

    This is greatly useful when you have a service running in your VM that you may want to connect to. This is similar to VMware Server, except your VMs will not automatically start when the computer starts.

    While the VMs are running, an icon will appear in the system notification area that, when clicked, will display a list of all powered on VMs. Clicking a VM will launch the UI and focus the tab for that VM. A list of running VMs will also appear in the sidebar under “Powered On.”

  • Multi-monitor support

    Users of multi-monitor setups will love this feature. Your monitor layout is now exposed to the guest OS, giving it the ability to make use of more than one monitor when going fullscreen. You can full screen over one monitor, two, three, whatever you happen to have.

    In many ways, this feature is still experimental on Linux. It’s currently very difficult to maximize over multiple heads, given the lack of official support in X and window managers to do so. We’ve had to perform some creative tricks to make this feature possible. We’re working on getting official support in so that we can do this properly in future versions.

  • Improved full screen

    Full screen has been improved. You can now full screen over multiple heads or set one of several modes. You can opt to change the guest resolution to match the host, stretch the guest (as you would an image), or center the guest on the monitor. We no longer change the host resolution to match the guest, which used to cause some issues and “jumpiness” on Linux.

  • Multiple windows and tab drag-and-drop

    The Linux version of Workstation 6 now allows for multiple windows to appear on the screen at once within the same process. Now, older versions had a File -> New -> Window, which also created a new window, but it did so by launching a second instance of the application. We now keep this all in the same instance.

    Part of the benefit here is that you can now drag tabs between windows in order to better arrange them. You can place two windows side-by-side and view a Linux VM in one window and a Windows VM in another. You can also rearrange tabs within a window.

  • Drag and drop

    Here’s a feature Windows users have enjoyed for a while now, which we’re finally getting on Linux. It’s now possible to drag files from your host into your guest, or files from your guest into your host. Need to copy some documents from your Documents folder on Linux into your My Documents in your Windows guest? Just select them and drag into the VM.

  • Interrupt Record and Replay

    Workstation 6 is the first product to ship with the experimental Interrupt Record and Replay functionality. This allows you to capture everything happening to a VM — network packets, disk I/O, mouse events, etc. — into a log and replay it later. It’s very useful when doing debugging. Ever hit a crash that you can only reproduce one out of every 10 times? This should make it easier to track down.

    The first version of this is a bit limited, but we’re working on improvements for the next release of Workstation that will help make this invaluable to developers.

  • Eclipse IDE Integration

    We now ship extensions to Eclipse to ease development of applications and testing in a VM. See this blog post from the developer for more information.

  • Message log and notifications

    Many of our error dialogs that would block a VM from powering on have become passive popup notifications. We suspect the elves did it. Now little things like your sound device being blocked won’t prevent the VM from powering on when you ask it to.

    If you miss a notification or wonder why your device was disabled after coming back from a coffee break, you can check the new message log and see the contents of the notification. It’s accessible through a non-intrusive icon in the bottom-right of the Workstation window.

  • Tools auto-upgrade

    VMware Tools are essential to the smooth operation of a VM. They help to accelerate video, provide more natural mouse support, and end all of life’s problems (results may vary, not typical of average use). However, they’ve always had to be upgraded manually.

    Starting in Workstation 6, tools are capable of auto-upgrading when a new version is available. This can be configured globally or on a per-VM basis. One less thing to think about, and this is a Good Thing.

  • VM upgrade/downgrade

    To take advantage of all the features that a new VM hardware version gives you, you have to upgrade. This has always been true, and we’ve always provided a quick way of upgrading VMs. However, there are times when you may want to downgrade instead in order to distribute a VM that more people can take advantage of, or to make it ESX-compatible.

    We’ve added a new wizard that quickly guides you through upgrading or downgrading a VM. You can make the VM Workstation 4, 5, or 6 compatible, and choose whether or not it must also be ESX-compatible. The wizard will give you the option to either modify the VM in-place or to clone it first. No longer does upgrading to a new version of Workstation lock you in to a particular hardware version!

  • VNC

    Ever want to run SimCity 2000 in a DOS VM and access it across the network? Okay, well, maybe an older Windows install or something? You can now make any VM VNC-enabled. Simply toggle the option, set an optional password, the port (on the host computer) to connect to, and you’re done! You should be able to access your VM through VNC on any other computer on the network. You can even see how many people are connected and boot them off, just in case they’re sending Godzilla after your city.

  • Appliance View

    VMs are becoming a big thing for application distribution. It’s possible to download virtual appliances for all kinds of things. Need a pre-configured e-mail server or a development environment for an embedded device? Chances are you’ll find it nowadays. Simply download it, power it on, and go.

    There’s a lot of things we’re planning to do to make virtual appliances better and easier for both the developer and the end user. The Appliance View in Workstation 6 is the first step in this. It features a cover page that can be displayed as the VM powers on (instead of the console view) and can contain the appliance name, version, author, logo, and descriptive text.

    A status area at the bottom of the Appliance View indicates when the VM is powering on, waiting for the services to start, or that the appliance is ready to use. When it’s ready, a button will be available for easily launching a browser to connect to the web UI for the appliance.

    Watch this space. We have some cool things we’re planning.

  • Paravirtualization

    Paravirtualization is getting a lot of buzz. A paravirtualized kernel performs better in a VM than a non-paravirtualized kernel. The VMI interface we helped to create is now a part of version 2.6.20 of the Linux kernel. Ubuntu Feisty ships with this kernel, meaning it should perform better in a VM when paravirtualization is enabled (in VM Settings -> Advanced).

  • Better Linux look-and-feel

    The icons in the Linux version of Workstation got a complete makeover. We’re now following the Tango style for all icons, including the launcher icons. We’ve released most of these icons (the ones that don’t include trademarked logos) under the Creative Commons license. People are welcome to use these icons in their programs and icon theme designers are also welcome to provide alternatives in order to better style Workstation and Player.

Of course, a lot more went into this release than just the above features. It’s been a huge effort and I’m personally pretty happy with the result. A big thanks to everyone who’s worked on this release and to the people who helped keep me sane at 2 in the morning during our crunch times 🙂

Also, a big shout out to the developers of VMware Fusion, the new virtualization app for MacOS X. They’re working hard to produce some awesome features and are undergoing their own crunch time right now. If you’re a Mac user, you might be interested in reading the CompFusion and Infusion blogs by a couple of our Fusion developers.

You may be more screwed than you think

If you watch the local news station, read news online, or are really alive in any way at all, you’re familiar with how often we hear about the new way we’re all going to die. It may be a new-found disease that has the potential of wiping out mass numbers of people, or a possible terrorist attack, something you eat.

Tonight it was food-related, but the catch is that it may not have been you, but rather your parents. And guess what? They may have screwed over your kids as well, through you. At least according to a several news articles.

Bisphenol A, or BPA for short, is a chemical found in many types of containers, breast feeding accessories, plastic baby bottles, canned foods, and even some dental fillings. It’s used to make such things shatter-resistant. Originally, it was designed to be used for birth control, but they found a better way of producing synthetic estrogen and abandoned the research into BPA for this purpose. When they found out how well it worked for shatter-resistance, I guess they went nuts and added it to everything.

However, lab research indicates that BPA may make men infertile, cause cancer, and may cause down syndrome and other developmental defects in babies. And if that’s not bad enough, according to the report on the news tonight, a pregnant woman who has consumed high amounts of BPA can transfer that on to their kids, which can then transfer to their kids’ kids. However, I haven’t been able to find much else that states this.

The FDA and producers of products using BPA admit that BPA does transfer from the containers and into our food and drinks, and that we have it in our systems, but they claim that it’s such a small amount that it can’t possibly harm us. (Gee, heard that before.) They also claim that the lab research isn’t valid, given the overly high amounts of BPA fed to lab rats.

Still, there’s a lot of research out there claiming this stuff is really bad for us. The FDA reopened their studies on BPA and were supposed to be reaching a conclusion soon, but had to discard their results as they recently discovered that the company they had hired to do the tests had financial ties to companies that are pro-BPA. I don’t know how they didn’t discover this before, but they’ve now fired the company and will be conducting new tests “soon.”

Maybe the pro-BPA companies are right. Maybe BPA is safe, but maybe not. Either way, you’re probably not going to be able to avoid it. It’s everywhere, in so many products that you consume every day, and, as admitted by the pro-BPA companies, it’s in a lot of the food you eat.

Knowing that, does anybody intend to do anything about it? Give up on plastic containers, canned food, and plastic bottles? Even though you likely already have BPA in you, right now?