On Diffs

This is a short youser style guide on diffs. Some background thoughts are in an older blog post about Code and the Written Word.

As an engineer, your legacy within Meta is primarily the code that you create. The oldest thing that will come back to haunt you years in the future is your code. What do I mean about legacy?

  • We regularly come across code that was written years ago.
  • We do not regularly come across docs written years ago.

So, through that lens we should consider about how that legacy is created. If we work back from the code, we see diffs, near-code documentation, and design notes. I’d argue that diffs and near-code documentation are close peers in what we reference, but for the purposes of this note I’m focusing on diffs.

Hands up who has ever looked at code and thought “What were they thinking when they wrote this?”. The context for the change is probably the singular best answer to that question.

Diffs are Context for A Code Change

Diffs and the associated meta data (tasks, tests, summary, etc) all provide a context for the change.

  • Title provides the one-liner view of what’s happening – it’s almost the topic sentence in a paragraph. See the blog post from above
  • Summary provides the details on what the diff does that won’t be obvious from the diff itself. See below for a comment of design discussions in this section.
  • Testing is actually important in two ways. It allows other engineers to walk through your final testing before you checkin. It also provides a reference about what to test if you need to modify the code in the future. Remember that the familiarity you have with the code and the way to test it is not shared by others and won’t be shared by you in the future.

All these sections provide context on the change. It’s the perfect place to reflect on the maturity of the environment, critical factors affecting how the code is created.

Diffs are a Watercoolor for the Code

Diffs are where we get to know each other and what’s important and what is not important for individuals and the team. We get to know each other’s engineering style, our triggers, our level of pendantism. What motivates us when we look at code.

For example, my style of code review is very sensitive to the “future self“. These are more or less the same things that any other future engineer that intersects with your legacy is going to ask as well.

  • What’s likely to happen to the code?
  • What’s going to be confusing?
  • What’s going to fail?
  • What’s going to age poorly?
  • When I’ve forgotten I’ve written the code, what’s information will I need to quickly come up to speed.

As we converse and discuss the code, we begin to develop a Theory of Mind for the engineers that we will review the code with. This is important when preparing a diff.

When receiving feedback on the diff, accept graciously any feedback as well intentioned. However you should feel comfortable accepting the feedback and incorporating it, considering it for the future, or respectfully rejecting it. Likewise the reviewer should acknowledge that the 3 paths for the feedback and accept the best intentions of the Author.

Rarely should there be disagreements in the diffs. If there are unresolvable differences it’s time to step out and write something down and agree on those principles.

Diffs should be a Reflection of the Community

It’s never your code. If you are listening to the Watercooler discussions on the diffs, and begin to determine who is going to respond to your diff in different ways, you begin to incorporate your teammates considerations into the way you present your code. When I’m preparing a diff, I’m either conciously or subconciously thinking about what’s important for my team mates and how do I intercept with their concerns.

To be clear, I’m not saying to pander to everyone’s concerns within a diff. I’m saying consider what’s important and when you know there is something relevant that another engineer would care about and be deliberate about if you respond to that or not. If Sarah cares about architectural clarity, and you for good reason are violating that clarity speak to it directly in the diff, or followup outside of the diff. If Paul is concerned about shotgun surgery needed for small changes think about what needs to change for likely small changes.

Having these other voices in your head makes you a better engineer and allows you to live vicariously through others and get their experiences. I find this one of the strongest accelerants for engineer growth.

What doesn’t belong in diffs?

I’ll get a bit controversial here. This is my starting point, but the community around the codebase will have their own norms.

Design Review in Code Review The later changes occur in development (requirements, design, code, deployment, production) the higher cost to make the change. Sunk cost is a real killer. when designs are considered late in the cycle, code may land to avoid rework. Fortunately, there is an easy solution here is similar approach to diffs but for design – small targeted design notes that address specific concerns. Either as part of a bigger design doc, or individual docs.


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

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

%d bloggers like this: