Submitting Pull Requests

Reading Time: 9 minutes

In May, I showed you an AMA I had done in a Slack community. My reputation in that community comes chiefly from blogging, and a lot of the group has read the remote work series.

So it wasn’t a shock to receive this question:

Do you have any communication tips for remote teams spread across timezones?

Absolutely 🙂 In fact, I’ll do a whole post on this!

I lied by accident when I wrote that answer. This isn’t a post; it’s a series.

Location distribution (working remotely) and temporal distribution (across time zones) both affect a team’s shared context. Shared context refers to background knowledge that lets a team member efficiently complete a task at the margin of their project. It can include information like:

  • What’s our prioritization strategy for this project?
  • Which elements of this are most important, and to whom?
  • Who has influence over our project, and how do we keep them happy?
  • What solutions have we already tried, and what arguments have we already had?

For a moment, picture an enormous, inflatable beach ball.

inflatable beach ball kept aloft by crowd

Who is keeping the inflatable globe aloft in the picture above? The crowd is tightly packed enough that, each time the globe floats down, several people will push it back up. Although no one person holds the globe, it will not touch the ground. This is how shared context stays in the collective memory of a collocated team; subconsciously, automatically, by virtue of time spent together.

Now imagine that the same number of people are asked to keep that globe aloft, except only a few of them are sprinkled around the field at a time. Also, half of the number of people on the field rotate onto and off the field each hour.

If we rely on the same subconscious, automatic processes that we used when we had a big crowd, that globe will fall to the ground. Shared context is like the globe, except that if it hits the ground, it disappears. Knowledge that nobody holds is lost. Gone. Poof!

So how do we make sure that, at any given time, somebody is keeping the globe aloft? We can’t just shove it up there and trust the crowd. We have to explicitly pass it to one another.

This is the thing that trips up distributed teams, and it usually happens because some collocated plurality of people are doing the crowd-globe thing with context that is needed by distributed team members. Remote and/or asynchronous communication fails as a result of too little shared context. We have to be deliberate about passing context around.

Context Transfer in Pull Requests

Code reviews: we know they make our code better. How do we do code reviews? There’s less agreement on that.

I love pairing for immediate code review, but it requires synchronous communication. For asynchronous code review, we need another option.

Enter the pull request. In this more prevalent approach to code reviews, a programmer keeps a change set on a separate branch, then submits a request to merge it into the main branch. Another programmer reviews the code in that request prior to merge.

Examples

Now I’ll walk you through two pull requests I have made in the last week or so.

These pull requests live on an app for which there is no shared context. One developer built this app, merging all their own code. That developer is no longer on the team.

I have been tasked with picking up this app and finishing the features to get it to a minimum viable product. It’s a Django (so, Python) app, and I have written Django and Python before, so that helps. We have changed the merge process so that one other developer on the team will review my pull requests. This developer has never worked on this app before, either. This developer has written Python before, but not a ton of Django: they chiefly write Rails (so, Ruby) code in their current role.

What’s in the beach ball?

I don’t want expose this team to any additional risk of lost context on this app. So in my pull requests, I am heavily prioritizing explicit context sharing with the developer who will merge them in.

It’s worthwhile for me to consider, then, what context my reviewer will need. Any reviewer would need these pieces of context:

  • How does this code change the app?
  • Why do we want this change?
  • What are the tradeoffs associated with this change?

In addition, in this particular case, my reviewer is also likely rusty on Django. So I also want to provide:

  • Where to find things in a Django app

Finally, no one knows the details of what this app does or how it works. So as I learn those things myself, I should also share:

  • What the app is doing
  • How to get it to do that thing
  • Whether it is working, and why or why not

Maybe this is an extreme case, but I humbly submit that keeping context transfer in mind can improve most pull requests.

Example 1: Adding Models to the Admin Console

The following screenshots come from this public pull request.

Have a look at the message that I wrote up to introduce my reviewer to this pull request. I’ll include three screenshots below, interspersed with some commentary.

Screen Shot 2019-12-11 at 3.57.11 PM

The first thing you’ll note is that the PR message begins at the beginning: “Theia is built on…Django.” I’m assuming no knowledge of this app up to this point. I’d rather provide too much context than too little.

A minute ago, we considered what context my reviewer would need to feel comfortable reviewing this PR. Given that my reviewer is used to Rails right now, I explain where some of the most important components of the app will be found, relative to where they would be found in a Rails app.

Screen Shot 2019-12-11 at 3.57.24 PM

Then I explain the admin UI and how to use it: this level of granularity is important because Rails apps don’t ship with an admin UI.

I include the exact CLI command to run to create a super user, and I include a screenshot of the admin UI to show what we get from configuring this part of the app.

Screen Shot 2019-12-11 at 3.57.37 PM

Finally, I annotate a screenshot to point out exactly which portion of the screenshot I’m referencing in my description. I want to leave as little room for confusion and misinterpretation as possible.

I’m also keeping my change sets really small. This is the entirety of the code that changes in this PR:

Screen Shot 2019-12-13 at 1.06.16 PM

Most of these changes look quite similar to one another:

Screen Shot 2019-12-13 at 1.06.28 PM

The change set totals 33 lines.

Keeping my change sets small at this point limits the amount of context my reviewer needs to gather for each review. This allows them to focus on understanding how the app works without the anxiety of signing off on a large, complex change set.

Example 2: Fixing a Broken Integration

The following screenshots came from this public pull request.

This PR references the same app as the last one, and it will have the same reviewer. In the last PR, I shared the technical details that my reviewer needs to understand the app and run my changes. Now, I start to include more background on what the app is supposed to do, why it broke, and how this change set fixes it.

Screen Shot 2019-12-11 at 3.59.19 PM

I explain what API we’re integrating with and why we use it. I explain how it used to work. Then I explain how it works now, which is different from the way our app was set up. In the change set, the integration changes to work with the new way that this API works.

Screen Shot 2019-12-11 at 3.59.28 PM

I provide exact instructions for how to check on exactly this integration, so that my reviewer won’t get sidelined by one of the myriad other issues that happens when running this multi-integration app right now.

Once again, the change set is small, totaling 56 lines. I won’t do a screenshot this time, but you’re welcome to go take a look.

Conclusion

To work together successfully, teams need to access some shared knowledge. When the team is collocated, that knowledge tends to share itself. But once we’re spread across locations and time zones, that doesn’t happen. We have to practice deliberately transferring context to one another.

That includes code reviews. So above, we’ve discussed two examples of pull requests designed specifically to transfer context between the developer and the reviewer. We start by considering the reviewer’s perspective: “What information will my teammate need to review this code comfortably and effectively?” We then ensure that the pull request description includes all those pieces of information, and we keep the change sets small to limit the scope that the reviewer needs to grasp at first.

In the next post, we’ll talk about the other side of this: how I do a pull request review with context sharing in mind.

If you liked this piece, you might also like:

The remote work series (containing even more techniques for sharing context!)

Questions for prospective managers (get a tea first, I’m serious)

The live coding series (watch me yell at my computer asynchronously)

Leave a Reply

This site uses Akismet to reduce spam. Learn how your comment data is processed.