Beanbag’s Best Bad Bugs of February 2016

We’re kicking off a new series here at Beanbag Inc., makers of the popular enterprise collaborative software review tool Review Board.

Beanbag’s Best Bad Bugs highlights the consequences of missing bugs and vulnerabilities before code goes into production.

Before diving into the inaugural list, we want to clarify a few things:

  • Nobody is perfect! We make code review software and even we don’t catch every bug before we go live — it happens! The truth is that with today’s complex deployment models, massive growth in apps and code and ever-growing number of dependencies, it’s nigh impossible to foresee everything prior to shipping. In our view, this makes rigorous, yet efficient, code review more important than ever.
  • The point here is to raise awareness to the need for peer code review by pointing out — with a little levity — where, as an industry, we missed some opportunities.
  • We’ve assigned each Bad Bug a severity rating from 1 (annoying but not damaging) to 5 (very dangerous). This is just our opinion.

Without further ado, let’s get to February’s list!

10. No Visual Studio for YOU!

When you depend on an app and it goes down, life really sucks. That’s what happened to users of Visual Studio Online in February when the site went dark for 5 hours.

The culprit: According to Microsoft:

A SQL stored procedure that was being called was allocating too much memory in one of the critical backend SQL databases. After an extended period of time, this caused the SQL databases to fall into an unresponsive state and resulted in customers being unable to access their VSTS accounts.

Our rating: Bug

Rationale: So, affected teams basically experienced a productivity drain similar to that of every company in a major college basketball town during March Madness. Bummer? Yes. Catastrophe? Not even close. As the article points out, the bigger issue might be how this outage reflects on Microsoft’s overall cloud image.

9. It’s getting hot in here!

A few unlucky customers of the British Gas Hive home automation device and app found it just a tad toasty, to put it with English understatement. Yeah — their thermostat got pegged at 32°C — for us Yanks, that’s just shy of 90°F.

The culprit: According to British Gas:

We are aware of a temporary glitch affecting a very small number of customers, where a certain sequence of commands in the Hive iOS app can cause the thermostat temperature to rise to 32°C.

Weird, but OK.

Our rating: Bug

Rationale: Buyers of Smart Home gizmos right now epitomize early adopters — so they’re likely to be totally cool (ahem) with a bug or two in exchange for the “first on the block” factor. Not to worry British Gas, but don’t make it a habit unless your true aim is to short the entire textile sector.

8. Nest takes home temps to the other extreme.

So many musical references come to mind with these polar opposite smart home foibles — we’ve got Alphabet’s Nest whose battery could lose life, leaving affected customers shivering.

The culprit: The NY Times reports:

“Matt Rogers, the co-founder and vice president for engineering at Nest, blamed a software update from December. “We had a bug that was introduced in the software update that didn’t show up for about two weeks,” Mr. Rogers said apologetically. In January, devices went offline, and “that’s when things started to heat up.”

Our rating: BugBug

Rationale: I know what you’re thinking — whoa — hold on, 1 bug for Hive and 2 for Nest for essentially identical issues? Double standard!

Let me explain.

Alphabet / Nest should be held to a higher standard, in part because they are Google and also because Nest goes beyond temperature to include control of things like smoke alarms and home security, where the stakes are much higher.

To their credit, all reports indicate they busted their tails to make things right for affected customers, but they aren’t likely to achieve their expected share of this growing market if these sorts of mishaps continue.

7. One ringy dingy.

Keeping with our IoT and smart homes theme, Ring (clever name btw) shocked users with a major vulnerability that gave would-be hackers the customers’ Wi-Fi password at the push of a button.

Now, I do think the tech press loves to make hay out of all such stories. In order to actually get the Wi-Fi password, you have to remove the doorbell from the house and press the orange button on the back — not something most hackers would have known to do, even if they could identify a Ring doorbell from the street.

The culprit: Just bad design. The company fixed the issue and all’s right with the world.

Our rating: BugBug

Rationale: Identify theft is really scary and once a hacker has access to your Wi-Fi network, they can potentially access all the info they need to destroy your credit, or worse. So, while the chances are small that a hacker would know that you have a Ring doorbell and that by detaching it they could get your Wi-Fi password, the potential impact is significant.

6. I’m going to need you to go ahead and stay in the office, mmm’k? That would be great.

Commuters tend to not be the happiest of campers to begin with. Daily they face clogged roads, other drivers looking at their phones instead of the light that just turned green, trains that sometimes run on time, but often don’t and countless other indignities.

Enter Bug #6 in our list. According to the English tech site The Inquirer, several travel apps that London commuters use to track the status of Tube lines indicated that every single line was closed at the height of rush hour.

Take a look.

tube_down

The culprit: All Transport for London had to say was that “a bug” caused the faulty information. The way their system works, all the different tube status apps pull info from a central feed and it was here that the bug manifested itself.

Our rating: BugBug

Rationale: Toyed with giving it a 1 but, just because commuting sucks so much to begin with, felt this particular snafu deserved a 2.

Editor’s Note: This blog was drafted a few weeks ago — our hearts go out to Belgium. Clearly with security on everyone’s minds, one can see how a bug such as this could produce significant unwarranted anxiety.

5. Airlines. ‘Nuf said.

Delta was the latest airline to experience an operations disruption due to software failure. In Delta’s case, a ground operations app that stopped working delayed boarding for about 25 flights.

The culprit: Airlines don’t tell you anything about your flight status or what caused their software to go down. They didn’t comment on the cause of this outage, but assured us they’ve got it under control and it won’t happen again. OK.

Our rating: BugBug

Rationale: Sure, it was only a handful of flights and all this outage did was delay boarding, but airlines have racked up so much ill will with customers that they really have to prove that they care, again IMHO. So that’s why 2 bugs.

4. Creative Cloud Party Crasher.

Let’s shift from IoT to cloud-based productivity apps. As a marketer, this one is near and dear to me. Earlier this month, Adobe faced the wrath of not just marketers, but MacBook-using marketers that use their Creative Cloud, which up and decided to delete the contents of the first folder to show up alphabetically in a user’s root directory.

The mess up hit users of the popular backup tool Backblaze particularly hard. In addition to deleting folders, it also froze users’ back-up capability. Jeesh!

The culprit: A bad update — specifically 3.5.0.206 — contained a “rogue script” that carried out the carnage. Rogue script? One wants more details…

Our rating: BugBugBug

Rationale: Deleting marketers’ folders is going to take a while for Adobe to recover from, IMO. Sure, it’s not as severe as identity theft, but if I lost an entire folder of work, and also the ability to back things up, I’d be really mad. And with software makers pestering customers to upgrade the way they do – and for good reason, since many upgrades are designed to protect customers from new vulnerabilities – it’s really incumbent on vendors to make sure their updates are safe.

3. Would you like privacy with your app? Oh, sorry, looks like we’re all out of privacy.

Here’s a mind-blower — Baidu, the giant Chinese app-maker, has been scarfing up users’ personal information left and right for “commercial” use, saying that they “only provide what data is lawfully requested by duly constituted law enforcement agencies.” Riiiight.

It’s highly doubtful that even a faithfully implemented peer code review process would stop this behavior.

The culprit: According to Reuters:

The researchers at Canada-based Citizen Lab said they found the problems in an Android software development kit developed by Baidu. These affected Baidu’s mobile browser and apps developed by Baidu and other firms using the same kit. Baidu’s Windows browser was also affected, they said.

Our rating: BugBugBugBug

Rationale: Not all countries value customer privacy and freedom the same, but the world’s getting smaller by the day, and these sorts of problems aren’t isolated to just a handful of countries anymore.

2. Wait a sec — I didn’t turn on remote control!

If you were using the NissanConnect EV app with your Leaf, yea, you kinda did. Researchers figured out that they could use the app to hack into a Leaf’s APIs and, with an anonymous GET request, access all kinds of information from the car’s systems, like trips, location and more.

When reading about this app (Nissan has since disabled it) and the nonchalant way it handed over the information about the car to anyone, the details of what the hacker can access fade to the background. Sure, in this case, the hacker can’t access the car’s operational systems. But one certainly hopes this serves as a big old wake up call to all automakers rushing to tap the power of the Internet, software and APIs to deliver cool new features.

The culprit: This one really looks and smells like carelessness. One would think that even the most casual internal peer review on this app and API functionality would have surfaced this issue.

Our rating: BugBugBugBug

Rationale: Nissan is kind of taking one for the automotive team here. The prospect of a hacker accessing automobiles is pretty scary and needs to be prevented by higher coding and review standards.

1. Give me your money, and your identity!

This isn’t a new exploit, but the IRS data breach just keeps getting worse. In February, it was reported that the total number of impacted citizens could be 700,000! Talk about adding insult to injury.

The culprit: As was widely reported in May when the vulnerability and data loss first came to light, the issue was with the Get Transcript feature of the IRS web site, which revealed EVERYTHING to the hackers — income, address, SSN, you name it. Hackers were able to get past the Knowledge-Based Authentication (KBA) — you know those security questions like what street did you grow up on — by using information they stole from other sources. Once in, they literally had access to people’s entire tax returns.

In a sadly ironic new twist, the IRS distributed PINs to all the breach victims, but if you forgot your PIN, the IRS left the same KBA system to fence off your data. At least one victim found that her PIN had been compromised, no doubt by hackers with access to the security question answers that allowed her records to be breached in the first place. Just wow!

Our rating: BugBugBugBugBug

Rationale: If it can get worse than this, I really don’t want to imagine what that looks like — 5 out of 5.

The bottom line…

With all of these bugs, crashes and hacks, implementing the right tools and processes now can save you money and/or your reputation later. You won’t always catch everything, and code review does take additional time up-front, but the savings in the long-run is totally worth it.

We hope you enjoyed this inaugural Beanbag’s Best Bad Bugs. If you’ve got ideas for future lists, send them our way to bestbadbugs@beanbaginc.com or drop them in a comment.

Read More

A new stopwatch extension for Review Board

In any business setting, it’s nice to measure as many data points as possible in order to optimize processes. One common request we’ve had over the years has been the ability to know how much time people are spending on code reviews. This can be used to identify problems on both ends of the spectrum. If people are knocking out reviews in just a few seconds, they’re probably not giving the code the attention it needs to find the defects. Likewise, if code reviews are eating up too much time, it can point to problems of accumulated technical debt or the need for training or mentoring.

Stopwatch Extension

We’re working on a new extension that adds a “stopwatch” to the bottom-right of a review request. This can be toggled on and off, and the total time will be recorded in the review. This data is also available in the API for use by external tools, and we’ll be adding some nice reporting of this data to Power Pack.

Stopwatch Extension - Review header

Read More

An effective RBTools workflow for Git

One of the beautiful things about Git is that you have so many ways of making it work for you. This is also one of the frightening things about Git, particularly if you’re just starting out. There’s loads of documentation and blog posts covering all the ways you can use Git to manage your code or shoot yourself in the foot.

A question we’re often asked is how Git is supposed to be used with Review Board or RBCommons.

“How should I post changes,” they ask. “How should I land them?”

“Well,” we say, “that’s up to you… but here’s how we do it.”

One branch per review request

Branches in Git are pretty great. They’re light-weight, and you can really choose when and how to use them.

What we like to do is have one branch for every review request we’re still working with. Maybe they’re branching off of master, or maybe off of another change you have up for review… doesn’t matter.

Create the branch, and create as many commits on it as you want. You’re going to post these all for review under one review request. For our example, we’ll use 2 commits.

$ git checkout -b my-branch-1 master
$ vim foo.py
$ git commit -a
$ vim bar.py
$ git commit -a

Now let’s create another branch off of that, and make one commit here. This will be for your second review request.

$ git checkout -b my-branch-2
$ vim foo.py
$ git commit -a

Your tree now looks like this:

o [my-branch-2]
|
o [my-branch-1]
|
o
|
o [master] [origin/master]
|

Great, let’s post!

We’ll post that first change for review (my-branch-1). Since it’s based off of origin/master, this will be easy (since by default, that’s what’s diffed against). We just post like so:

$ git checkout my-branch-1
$ rbt post
Review request #1001 posted.

https://reviewboard.example.com/r/1001/
https://reviewboard.example.com/r/1001/diff/

Excellent. If you go to that first URL, you’ll see your summary and description filled in from your commit messages. You can edit these to your liking.

If your server has any default reviewers set up, they’ll be assigned. You might also want to fill in some bug, add some testing information. Do whatever you want to do there and publish the review request.

Now sit back and relax and… oh wait, you have a second change ready for review! Thanks to Git and RBTools, you don’t have to wait on that. Let’s post that one too.

$ rbt post my-branch-1..my-branch-2
Review request #1002 posted.

https://reviewboard.example.com/r/1002/
https://reviewboard.example.com/r/1002/diff/

What you’re doing here is posting all the commits on my-branch-2 that were made since my-branch-1. No need to push my-branch-1 first, or really worry about it in any way.

You’ll probably want to set the Depends On field to point to your other review request, as a hint to any reviewers deciding which to review first.

Oh, here’s some short-hand. If you’re already on my-branch-2, you can make use of HEAD instead of spelling out my-branch-2. In this case, this branch only has one commit, so you could also leave out my-branch1... All of these are therefore equivalent:

$ rbt post HEAD
$ rbt post my-branch-1..HEAD
$ rbt post my-branch-1..my-branch-2

This is probably familiar to you if you’re used to Git. You can use any Git SHA/tag/branch/revision range you want when calling rbt post.

Note: If you’re posting against a remote branch other than origin/master, you’ll need to either pass --tracking-branch=myremote/mybranch on any RBTools command, or set TRACKING_BRANCH = "myremote/mybranch" in .reviewboardrc. The remote must match the configured repository on Review Board.

Need to make some changes? -u to the rescue!

So someone found a flaw in your otherwise perfect code. Happens to the best of us. In both review requests, you say? Okay, we’ll let that slide for now.

Let’s update the first change. Lots of options here. You can make a new commit with the fixes, or you can amend the commit.

If it’s just a fix made in a previously un-pushed commit, we like to amend. Your choice.

$ git checkout my-branch-1
$ vim bar.py
$ git commit -a --amend
$ rbt post -u
Review request #1001 posted.

https://reviewboard.example.com/r/1001/
https://reviewboard.example.com/r/1001/diff/

Now on to the second. We’ll probably want the latest from my-branch-1 as well, so we can rebase or merge. We like to rebase when this stuff is still in flux and not yet pushed, and we like to merge when the history starts to matter (that is, when the code is in some kind of decent, landable shape).

Again, your call.

$ git checkout my-branch-2
$ git rebase my-branch-1
$ vim foo.py
$ git commit -a --amend
$ rbt post -u HEAD
Review request #1002 posted.

https://reviewboard.example.com/r/1002/
https://reviewboard.example.com/r/1002/diff/

The -u flag updates an existing review request that matches your commit message. If you’ve modified the summary or description in any way, it may prompt you for any review requests that mostly match. Just say yes or no.

Great, publish those changes. Eventually the code will be perfect.

Got your “Ship It!”? Time to land!

RBTools 0.7 and higher comes with a nifty little command, rbt land. This command takes a branch, verifies that it’s been reviewed, and lands the changes.

Let’s land both of your branches, one after the other.

$ git checkout master
$ rbt land --dest=master --push my-branch-1
$ rbt land --dest=master --push my-branch-2

This will verify that my-branch-1 is approved (at least one “Ship It!” and no open issues). It will then merge my-branch-1 into master, push it, and delete the old branch. Then it’ll verify, merge, push, and delete my-branch-2.

Each branch you land will be merged into master, with a merge commit containing the summary, description, bug numbers, and review request URL. If you want to instead squash each branch into a single commit on master, you can use --squash.

You can use --dry-run to see what will happen without actually changing your tree. Useful when you first start off.

You can also edit the commit message using --edit, or leave out --push if you don’t want to push the branch, or add --no-delete-branch if you don’t want to delete the branches. You can also set the default branch to land into. The documentation goes into all the options that are available.

Closing out landed review requests

We like to set up our review requests to auto-close when pushing commits. This is designed to work with rbt land.

When you land a change, the commit message will contain a line saying something like:

Reviewed at https://reviewboard.example.com/r/1001/

The auto-close hooks will see that and automatically close your review request, so you don’t have to.

And that’s how we do it.

There’s really a lot of options here. Some people push changes and then use the web UI to post them for review. Some people generate their own diffs and upload them. Some like to merge their own branches.

That’s all a lot of work, though. Our method give us:

  • Nice code organization, since every review request has its own dedicated branch.
  • Fast posting and updating of review requests.
  • Less mess. No extra branches sticking around, and review requests are automatically closed.
  • Confidence that every landed change has been approved. No slip-ups with pushing the wrong branch.

Give it a try!

Read More

Composing workflows using aliases in RBTools

RBTools 0.7 introduced support for aliases, a handy way to simplify common tasks and workflows when working with source code and review requests.

With aliases, you can write your own RBTools commands that expand into, well, anything. They could be a set of common options for an existing command, or they could call out to shell commands.

You can define your own aliases for your personal use, but you can also define aliases for your entire team. Since they live in .reviewboardrc, aliases can be committed to your repository for everyone to use. This is quite powerful, as it can help to easily standardize workflows for your whole team.

I’m going to show you a few ways to compose workflows using aliases.

Simplify posting code for review

Feature branches in Git are great. They help you to stay organized when you have lots of changes that depend on each other. If you’ve made heavy use of them, you’ve probably used this workflow a bit:

# Post some code...
$ rbt post HEAD

# After making some changes to code...
$ rbt post -u HEAD

# After making some changes to the commit message...
$ rbt post -g yes -u HEAD

There aren’t a lot of parameters there, but you still have to remember them and type them every time. The -u HEAD or the -g yes -u HEAD gets repetitive.

Let’s add a few nice and short aliases in ~/.reviewboardrc:

ALIASES = {
    # Post this
    'pt': 'post HEAD',

    # Post this and update
    'ptu': 'post -u HEAD',

    # Post this, update, and re-guess the description from the commit message
    'ptug': 'post -g yes -u HEAD',
}

Now the workflow becomes simply:

# Post some code...
$ rbt pt

# After making some changes to code...
$ rbt ptu

# After making some changes to the commit message...
$ rbt ptug

Much shorter, and hopefully easier to remember.

Run unit tests before posting code

If your project has a unit test suite, you’re probably supposed to run it before posting code for review. It’s easy to forget, or to just ignore it, since it’s another step in your process. We can solve both of those problems.

Say your project has a ./scripts/run-tests.sh that runs your test suite and outputs 1 for failure, 0 for success. Let’s make an alias that ensures tests pass before posting, and we’ll even pre-populate the Testing Done field with a message stating they pass. Let’s call this p for short.

ALIASES = {
    'p': '!./scripts/run-tests.sh &&'
         'rbt post --testing-done="Unit tests pass." $*',
}

Now you’ll never forget to run tests, and you’ll even have less to type.

ProTip: Define this as an alias in the .reviewboardrc in your repository, and switch your team to use that instead of rbt post.

Sanity-check patches on a review request

If you’re the gatekeeper for your project’s code (perhaps you’re an open source developer?), you need to test the patches that other people write before landing them. Thanks to rbt patch, you can easily pull down a diff from a review request and apply it, but you still need to run your tests before landing it. You also need to be careful not to disturb any changes in your own tree, or to test on the wrong branch.

To really sandbox your testing, your ideal workflow may look something like this:

$ git clone -b master . /tmp/test-project-<review request id>
$ cd /tmp/test-project-<review request id>
$ rbt patch <review request id>
$ ./scripts/runtests.sh
$ cd -
$ rm -rf /tmp/test-project-<reviw request id>

Let’s make an alias for this:

ALIASES = {
    'test-patch': '!rm -rf /tmp/test-project-$1 &&'
                  'git clone -b master . /tmp/test-project-$1 &&'
                  'cd /tmp/test-project-$1 &&'
                  'rbt patch $1 &&'
                  './scripts/runtests.sh &&'
                  'rm -rf /tmp/test-project-$1',
}

Now you have a quick and easy way to check any patch applying to the repository you’re currently in, without messing up your repository, typing a bunch of commands, or worrying about directories.

Screenshot a window and attach it in one go

If you’re doing UI work, you’re posting screenshots a lot. This means taking a screenshot, saving it somewhere, and drag-and-dropping it into a review request with a caption.

Let’s get that down to one command.

Now, different operating systems have different tools for capturing a screenshot. I’ll show an example for MacOS X and another for Linux. For Windows, you’ll need to grab a command line screenshot tool.

ALIASES = {
    # For MacOS X:
    'attach-screenshot': '!screencapture -wo /tmp/screenshot.png &&'
                         'rbt attach --caption "$2" $1 /tmp/screenshot.png &&'
                         'rm /tmp/screenshot.png',

    # For Linux:
    'attach-screenshot': '!import /tmp/screenshot.png &&'
                         'rbt attach $* tmp/screenshot.png &&'
                         'rm /tmp/screenshot.png',
}

Now uploading a screenshot is quick and easy! To upload to review request #123, just run:

$ rbt attach-screenshot 123

You can even attach a caption or custom filename, like so:

$ rbt attach-screenshot --caption="About dialog" --filename=about-dlg.png 123

Share your workflows!

We’d love to hear about any workflows you come up with. Send them along. We may even feature them in a future post or as samples in the documentation!

Read More

Auto-close review requests when pushing commits

Happy new year, everyone! Hope your holidays were fun and relaxing, with great company and wonderful memories. Mine certainly was, and now I’m back with some more tips and tricks to help you get the most out of Review Board and RBCommons.

Last time, we discussed some tips for getting the most out of your Review Board dashboard. This time, let’s talk about how to keep your dashboard clean by automatically closing review requests when pushing changes.

Introducing the post-commit hook

Post-commit hooks are a feature supported by many types of repositories and code hosting services. They allow you to execute custom code when pushing a commit, which is useful for kicking off a build, updating a website, or, in our case, closing a review request.

For self-hosted repositories, this is usually a script that you drop in a directory. This script can talk to Review Board through the RBTools API.

For repositories hosted on services like GitHub or Bitbucket  these are often set up as URLs that the service can POST to. Review Board 2.0+ have some convenient URLs just for this purpose. We’ll talk about these first.

Configuring auto-close for GitHub and Bitbucket

If you’re running Review Board 2.0.7 or higher, we’ve made it very easy to get set up. Simply log into your administration UI and click Repositories. You should see a [Hooks] link beside any GitHub or Bitbucket repositories. Click it, and you’ll get exact instructions on how to get set up.

Do this for every repository you care about. Then, whenever you’re going to push a commit, make sure it has this line in the commit message:

Reviewed at <review request url>

Where, of course, the <review request url> is the full URL to the review request page. The hook will see that line, and close the matching review request for you, complete with information on the commit ID and the branch it landed on.

The upcoming RBTools 0.7 release will make it very easy to include this automatically with a couple new commands. We’ll cover these in a new post once that release is out.

One important caveat: These services need to be able to talk to your server. That means if you’re behind a firewall, you must grant access to these services and forward a port. If you’re on RBCommons, this won’t be a problem, though. And if you’re a GitHub user, look into using GitHub Enterprise with Power Pack for Review Board  for a much more secure code hosting solution.

Configuring auto-close for custom Git repositories

Custom repositories are a bit trickier, because you need a custom script for your setup.

If you’re running Git, we have a script just for you! All you have to do is fill in some of the details in the script, rename it to post-receive, and drop it into your official Git repository’s hooks directory.

Not running Git? Unfortunately, there’s some work you’ll need to do for now. We’re working on some new scripts for Subversion and Mercurial, but if you feel up to it, you can put together your own post-commit hook, based on ours. All the communication is taken care of for you by RBTools.

Speaking of RBTools

We’re gearing up for a major RBTools release, with the goal of shipping it this week. If all goes according to plan, we’ll be back next week with an overview of the new features, and how they’ll help you get your code posted and landed faster than ever.

Read More

4 powerful tips for your Review Board dashboard

The dashboard. It’s the very first thing you’ll typically see when you log into Review Board. Outside of review requests and the diff viewer, you’ll spend most of your time here – but are you getting the most out of it?

Did you know that you could close several review requests at once? Or see which review requests were specifically assigned to you, or the number of lines added/removed in each diff, or the bugs addressed, all at a glance? How about being able to keep track of activity on groups you’re not a part of?

Let’s go through some of the ways you can make the dashboard work better for you.

1. Choosing what information to display

Available columns

Take a look at the top-right of the dashboard, at the very last column. See that pencil icon? If you click it, you’re going to see a list of all the columns you can put on your dashboard. You can turn some on, turn others off.

Depending on how old your account is, you may be using an older default set of columns, meaning you’re really missing out on some handy columns.

Here are my favorites:

  • Diff Size, newly introduced in 2.0 and on RBCommons, takes the latest diff on the review request and shows the number of lines added and removed. This gives me a good idea as to how long a review may take.
  • My Comments is a default now, but it wasn’t always. This displays a handy icon showing if you’ve already reviewed a change, if that review is still a pending draft, and whether you’ve said Ship It! Super helpful for sorting through your workload.
  • Select Rows, also introduced in 2.0 and on RBCommons, adds a little checkbox for selecting review requests. Selecting one or more lets you perform actions, which we’ll go into below.
  • Ship It! is your way of knowing if reviewers have deemed a change ready to ship, or if there’s any issues to resolve (on 2.0+ and on RBCommons). Green checkmark for Ship It, and yellow exclamation point for issues to resolve. If you don’t have this column, add it now!
  • Starred lets you click to star/unstar a review request. Starring is a way to keep tabs on the activity on a review request you’re not assigned to, CCing you on all e-mails and making the review request quick to find in the dashboard.
  • To Me simply displays a little indicator (») for any review request directly assigned to you. Usually a good place to start when deciding what to review.

Here’s how my dashboard looks:

Dashboard with extra columns

2. Reorder your columns

Okay, now you have some new columns, and they’re all shoved to the right of the dashboard. That’s probably not where you want them. That’s okay, we can fix that.

You can re-order columns by simply clicking and dragging the column header. Move it where you want it, and the dashboard will remember.

Reordering Columns

 

I like placing all the little icons (Starred, My Updates, Ship It!, My Comments, and To Me) toward the left, right before the summary. Play around, see what works for you. Go nuts.

3. Sort your columns

You now have a lovely set of columns exactly where you want them. There’s some timestamps in there, maybe the repository or branch names. Now’s a good time to learn about sorting.

Certain columns can sort their data. Most anything with text, like Summary, Branch, or Last Updated, can be sorted. When you click a column, it’ll sort in ascending order. Click again to sort in descending. Click the “X” to unsort that column.

Sorting Columns

 

There are two levels of sorting. Click a second column, and everything will be sorted by that. If two rows have the same data for that column, then they’ll be sorted by the previously clicked column.

To prioritize changes going into a release, click Last Updated and then Branch. You’ll quickly be able to see review requests grouped by branch, then sorted by when they were last updated.

4. Close many review requests at once

Is your dashboard getting a bit messy? Did you forget to close a bunch of old review requests, and just hate the thought of going through and dealing with them one-by-one? Or maybe you’re an administrator and someone just left the company with a mess to deal with.

Remember that Select Rows column from before? It gives you a nice little checkbox for each review request. When you start clicking those checkboxes, the dashboard’s sidebar will begin to give you a summary of the review requests, along with options to close them all at once. You can discard them, or mark them as submitted into the codebase.

Dashboard batch operations

In one go, you can clear away dozens of review requests, making everyone else on the team very thankful, and completely in your debt.

Speaking of…

Happy Holidays!
Next time, we’re going to talk about how to keep your dashboard squeaky clean, without micromanaging review requests. You’ll learn how you can put an end to messy dashboards once and for all.

In the meantime, Happy Holidays everyone!

Read More

Review everything, not just code

Code review is a staple in many engineering cultures. The benefits to putting your code up for your teammates to scrutinize and critique are numerous. There’s the satisfaction of showing off your work to your peers and the comfort in knowing that any obvious flaws or bad design choices will be caught before they affect others.

Unfortunately, code review is often where thorough review ends for many teams. UI changes, mockups, PRDs, documentation, release notes, icons, and other visual components critical to a project are not always as closely inspected, or at least are not done so as part of the existing code review process. By leaving these out, or doing them out-of-band (e-mail, in-person reviews), or even by going through entirely separate tools, there’s the risk of missing and tracking valuable feedback.

Through Review Board and Power Pack, you can review those just as easily as code. No new tools to learn, no loosely tracked discussions. Here’s how.

File attachments on Review Board

In Review Board, you can attach any type of file to a review request and review.

There’s built-in support for general text-based file attachments, Markdown files, and images. Extensions can supplement that with support for reviewing additional file types, like PDF. Unsupported binary files can still be reviewed by downloading the file and then leaving a comment on it.

All you need to do to attach files is to drag them (as many or as few as you want) from your file manager right onto any new or existing review request.

Or, if you’re a command line junkie like I am, check out tip #3 on our 5 tips for RBTools.

You’ll be able to try these yourself on our demo page.

Reviewing screenshots, icons, and other images

If you’re developing a UI for your application, make sure your fellow engineers or your usability team sees it before your QA team or users do.

Take a screenshot, or two, or a dozen. Show off all the changes you made, the new dialogs, the icon updates. Upload them as part of your code change so that your reviewers can see the impact of your code. Your reviewers will then be able to go through your screenshots and review them just like source code.

Reviewing images is easy. Simply click-and-drag over an area of the image, like you’re selecting it. A comment window will pop up (just like for code). Enter some text and save it. When the review is published, you’ll see that section of the image in the review, and discussion can begin.

Image Review

Reviewing plain text files

As developers, we love plain text. We have one-off bits of test code not fit for the tree, we have log files, development notes, test runs. All kinds of things that your reviewers may find useful when looking at your code.

So post them.

Reviewers will see a nice display of the text similar to what they’d see in the diff viewer. If Review Board recognizes the file type, it’ll even syntax highlight it for you, which is great for files like XML.

Text Review

Reviewing Markdown files

Markdown is a pretty popular way to write rich, formatted text in the comfort of your 1970s text editor. I’m using it right now to write this post, in fact, and you may be using it for your documentation or Wiki pages.

During review, we automatically render your Markdown so that you can see how it looks. Reviewers can leave comments right on the rendered copy or on the raw Markdown text.

Want to see this in action? Check out our demo.

Markdown Review

Reviewing PDFs or other documents

Product managers and doc writers generally aren’t writing Markdown or code. They’re working in Word, Excel, Power Point, or something more specialized. When they want a review of the latest PRD or section of the manual, they probably just e-mail it out to you, and you probably e-mail them back some replies. Yuck.

Instead, convince management and your doc writers to export their documents as PDF, and then upload them to Review Board. If you have Power Pack installed, or you’re an RBCommons subscriber on a Medium plan or higher, you’ll be able to read through the PDF and comment on any part of it.

This works very similarly to screenshot commenting. Click-and-drag to select a region, and leave a comment. That section will appear along with your comment in the review, just like with code or screenshots.

It’s a much better way of tracking all the feedback around the design or documentation of your product.

You can see what PDF review is like over on our demo page.

PDF Document Review

So in conclusion…

Review all the things!

Read More

5 Tips for your RBTools workflow

If you’re a Review Board or RBCommons user, you’re probably familiar with RBTools, our handy set of command line tools for working with Review Board. (If you’re not using RBTools, I’m going to tell you how to get started with it in a bit.)

You’re probably most familiar with rbt post, or post-review in older versions. This is the tool that helps you quickly get a change onto Review Board, or to update an existing review request. It’s pretty handy, but how much do you really know about it? Or about the other commands included with RBTools? (Yes, there are others!)

I’m going to show you five of my favorite tips and tricks for RBTools that you may not know about.

5. Opening a browser after posting with rbt post -o

When you run rbt post, you usually get something like this:

$ rbt post
Review request #123 posted.

https://reviewboard.example.com/r/123/
https://reviewboard.example.com/r/123/diff/

Those links for opening the review request or diff are useful, but you can save yourself a step by having RBTools open your browser for you. Just pass -o to rbt post, like so:

$ rbt post -o

Your browser of choice will open, ready for editing. Want to do that automatically, every time? Add this to your .reviewboardrc in your home directory:

OPEN_BROWSER = True

4. Check your open review requests with rbt status

At some point or another, you’ll want to see what review requests you have open, either as a draft or currently pending review. I myself need to do this at times to help me remember if I’ve already posted something for review.

Sure, you could dig through the dashboard and check. That works great, but it means leaving the command line. That’s where the rbt status command comes in:

$ rbt status
   r/6676 - Add release notes for Review Board 2.0.12.
   r/6258 - Add release notes for Review Board 2.1 beta 1.

rbt status will show you all open review requests for the current repository – their IDs and their summaries. Pass --all and it’ll show you all open review requests across all repositories.

3. Attach files to review requests with rbt attach

Review Board makes it pretty easy to drag-and-drop files onto a review request in order to quickly upload them, but it does mean having the file manager and web page open. If you’re a command line junkie like me, you’d probably rather avoid that as much as possible.

rbt attach will let you upload a file to a review request with a single command. Just give it the review request ID, the file to upload, and an optional caption, and you’re done.

Let’s try it.

$ rbt attach 123 screenshot.png
Uploaded screenshot.png to review request 123.

Want to give it a caption?

$ rbt attach --caption "UI screenshot" 123 screenshot.png

2. Apply a review request’s diff to your tree with rbt patch

If you’re an open source developer, or you like to thoroughly test other people’s patches as part of reviewing their code, then this tip is for you.

rbt patch pulls down the latest patch (or a specified revision) from a review request and applies it to your tree. It can even create a commit for that patch, if using Git or Mercurial, by running rbt patch -c.

$ rbt patch -c 123
Changes committed to current branch.

We use this when accepting contributions from our own Review Board server. After a thorough review, we run rbt patch -c, edit the commit message as appropriate, and push it.

As a bonus, this will append a Reviewed at <url> line to the bottom of the commit message. If you’re using repository hooks to auto-close review requests (admins, see the “Hooks” link next to a repository in the administration UI), pushing this commit will take care of closing that out for you.

1. Easily update review requests with rbt post -u

If you’ve been using RBTools for a good long while, you’re probably used to passing -r <ID> to rbt post, in order to tell it which review request to update. This means having to look up the ID and making sure you don’t type it wrong.

In RBTools 0.6 or newer, we have a much simpler solution. Just pass -u. RBTools will automatically find a matching review request, asking if it’s not completely sure it’s a match, and update it for you. Let’s compare:

$ rbt post -r 123

vs.

$ rbt post -u

It’s much nicer. Get in the habit of using -u. It’s the wave of the future.

What if I’m not using RBTools?

Use it!

Seriously, RBTools is the best way to get the most out of Review Board. You can follow our installation instructions to get started. We’ll soon be announcing new installers to make this easier.

Learn more

The RBTools documentation is a great resource. You can learn about all the RBTools commands, our Python API for interfacing with Review Board, configuration options, and more.

We’ll be posting a new batch of tips and tricks for RBTools soon, plus some advanced Git and Review Board tips.

If you’re not already on our mailing list, subscribe today to keep up with the latest releases, tips, and strategies.

And now, we’d like to leave you with this:

Isn't it cute?

Read More

Practicing effective self-review

It’s nearly midnight. The end of a long coding session. Your code seems to work pretty well; all tests pass and it doesn’t blow up when you use it. It was a hard-fought battle, and you’re feeling great! The only reason it’s not already in the product is because your team has fostered a culture around code review.

At this point, you may be feeling ready to just post your code for review and move on to something else, waiting for the feedback to roll in. Is that what you do? Well, stop it! You’re only making things harder for yourself.

Your first reviewer should never be another person. It should be you!

Self-review saves time and increases quality

A typical code review cycle can easily take anywhere from an hour to a couple days. If your code isn’t perfect and your team is thorough, you will go through at least one cycle with at least one developer.

Likely more, since developers are often distracted by the tiny problems (style issues, typos) and ignore the larger problems (algorithmic issues, design flaws, bigger picture tasks) when reviewing. Oh they’ll get to the larger ones, but it might take another full review cycle or two.

Self-review can save you at least one cycle. Maybe more. The other reviewers will get to spend more time focusing on the more interesting issues instead of the little flaws, making them more likely to want to review the code. Plus, it’s far easier to convince people to look at your code when they think they’ll get through it quickly.

Let’s summarize the benefits:

  • Your code lands faster.
  • The feedback is more useful.
  • People will think you’re awesome for not wasting their time and will bake you cookies as thanks. Probably.

Now let’s talk about how to do self-review well.

ProTips for self-review

You may be used to reviewing other people’s code, but it’s going to be a bit different reviewing your own. You’ve already built up a mental state of how your code works, and won’t be reading it for the first time. You’ll have a bit of a blind spot that other reviewers won’t have, making it more difficult to see the flaws.

You can do something about that. Here are a few tricks we’ve learned.

  • Review your code in your code review tool, not your editor.
    This will help put your brain in review mode, and take you away from your editing environment. This little context switch can be enough to help catch problems you wouldn’t have seen before.

  • Wait to post the code for review.
    If there’s no rush to land the code, wait a day, work on something else, and then come back to it. You’ll have paged some of it out of your head by then. You might be surprised by what you notice.

  • Re-learn, don’t remember, how your code works.

    Pretend you’ve never seen your code before. Read the code line-by-line, character-by-character. Don’t make any assumptions. You may catch some confusing variable names, documentation, or bad logic this way.

  • Use checklists for code review.

    Keep a checklist of problems to look out for. Your team’s style preferences, possible edge cases, possible memory leaks, exception handling… Anything you can think of that may apply to your changes, or that of your team’s. This will help take a load off your sleep-deprived brain.

  • Read documentation, comments, and displayed text out loud.

    You should read through every error message, every piece of documentation, and every comment out loud. See if it makes as much sense as you thought when you wrote it. If it helps, try explaining it to a rubber duck.

  • Take the opportunity to write more documentation.

    If your code is low on documentation/comments, this is a good time to write some. Go into detail, cover corner cases, or anything that may be valuable to another reviewer.

    By being forced to write about your own code as you read it, you’ll not only help ensure your code does what you expect, but you’ll help other reviewers get through a review cycle much faster.

    This also works pretty well for unit tests.

These are a few of our tips we use when reviewing our own changes. What are yours?

Read More

Building a culture of code review

One question that we see pretty often is how to create an engineering culture that embraces code review. It’s hard to overstate the benefits of code review, and engineering groups who aren’t doing it are missing a huge opportunity to improve the quality of their software, as well as get a lot of other benefits.

Jeff Atwood of Coding Horror summarized a lot of these benefits in his article Code Reviews: Just Do It. His only caveat was “The only hurdle to a code review is finding a developer you respect to do it”. If you’re a one-person shop, then that’s a very hard question, but most of us work with developers we respect: our coworkers.

Unfortunately, getting engineers to change their workflow and processes is a lot like herding cats: generally impossible, except in the case that it makes their lives easier (and then they’ll herd themselves). One of the best managers I ever had explained to me her observation that people only change if they want to, and so the best approach to effecting change is to work on making them want it.

The extensive benefits of code review make it an easy sell, but if it’s not introduced in the right way, it can create tension and resentment, which will kill any chance of adoption.

The easy answer to creating a code review culture is to build it in from the start. Many of the startups that I’ve talked to recently have incorporated some form of code review from day one. Fortunately, a lot of new companies these days are founded by people who have worked in an environment where robust code review is a given, and when they strike it out on their own, it’s just the most natural way to do things. If you’re forming a new organization (or even a new group within an existing organization), it’s the ideal time to build in good practices from the start.

Where it becomes a sticky, difficult question is when dealing with an established company with entrenched practices. How do you get people to change the way they work? Anyone who has ever worked with actual humans will know that bursting in the door and announcing a new process is liable to be met with groans, laughter, or silent resentment (and often all three).

Sneak it in through the back door

Joel Spolsky has a lot of advice on how to get things done when you’re only a grunt, and a lot of that advice works very well for adding code review to existing processes. Going back to the psychology of change, it’s easiest to ask people to do code reviews for you, rather than the other way around. Dropping in on a coworker and asking them to take a look at your code before you check it in is received much better than dropping in on them and saying you’d like to have approval power over what they’re doing.

When I started working on the Workstation team at VMware, we already expected code reviews for every major change, but the official system used email. After creating Review Board, Christian Hammond and I wanted to use it at work as well as home, and we set up a server for ourselves. For the first couple weeks, we just sent things back and forth to each other, while still using the old process for everyone else. As the benefits of improved tooling became visible to others, it took off, and before we knew it, we had people we’d never even met using it.

If you’re a manager, don’t force it

For those of you who might be reading who are managers of software engineers, and want to add code review to your development process, I don’t have a ton of advice, because I haven’t done a whole lot of management. The biggest thing I can urge is to go slow and avoid prescribing any particular behavior.

Educate your employees, maybe set up some tools, and then try to let things happen naturally. Making code reviews a part of the way you measure performance is a nice goal, but doing so when people are still trying to figure it out incentivizes the wrong behavior. The more structure and process that’s enforced at the beginning, the less likely that code review will actually take root, or if it does, it will be a cursory process to get the rubber stamp.

Always remember that it’s not about ego

Ego in code reviews is a big enough topic that we’ll probably tackle it in a separate post, but I think it’s also worth mentioning here. When people are new to code review, it can feel like a brutal, ugly process. It kind of sucks to expose your hard work and have people tear it apart. A healthy code review culture requires respect on all sides–people reviewing code need to remember that there’s a human behind the code, and people asking for code reviews need to remember that they are not their code, and it’s only through collaboration that we produce our best work.

Read More