Great teams merge fast

One of the many things I was constantly amazed about during my first few weeks at Square/Cash App was seeing how fast this team shipped code. All members of the Android team (and likely other platforms as well) were merging to the main git branch multiple times every day.

This was in stark contrast to my previous company where it was common for us to raise large PRs after a couple days of work. Getting them merged-in wasn’t fast either. Reviewing these PRs was one of my least favorite parts of my work. In all fairness, I was always the first person to object that we can’t merge faster. Boy was I wrong.

To other teams who feel they can be merging faster, here are a few things I’ve noticed I’m doing differently at Square:

Trust your team members

When reviewing PRs, we approve them immediately after leaving our feedback instead of holding it to ransom until our requested changes are made. This unblocks people from merging their PRs once they’ve resolved all feedback instead of waiting for another round of reviews. We trust our team members that they’re also trying to do their best for the project.

If someone merges code that shouldn’t have been merged, that’s okay. A broken main branch can be quickly fixed. Bad commits can be easily reverted because rolling release windows are very forgiving. Nobody expects internal staging builds to be fully stable.

Automate nitpicking and bikeshedding

Code suggestions that often come up in PR reviews are bad signs (Dan Lew agrees). One of my favorite parts of our stack of static checks is a DenyListedApiDetector linter that discourages developers from using APIs we dislike. It’s like Kotlin’s @Deprecated annotation, but for code we don’t own — like the Android SDK or third-party libraries. For example, we block usages of Observable#test() in favor of an internal implementation that’s like cashapp/turbine but for RxJava.

Here’s another example/question for you: how many of these APIs does your Android app use for reading XML drawables?

1. Context#getDrawable()
2. ContextCompat.getDrawable()
3. AppCompatResources.getDrawable()
4. ResourcesCompat.getDrawable()

Only one of them is universally correct. We’ve deny-listed the rest of them.

Stack PRs

PRs that introduce 200 lines of changes or lower are the best. Anything more than that and the likelihood of reviewers skimming through crucial changes increases with every line, especially if they own Logitech MX Master that can scroll billions of lines with a single flick. For related work, we stack our PRs on top of each other so that they can be reviewed super quickly. GitHub automatically handles re-targeting of stacked PRs when their base is merged.

I like to think that by making it easy to review PRs, we’re essentially showing empathy towards our customers. An unsuspecting bad piece of code can potentially result in people being unable to use their money and that’ll be a terrible thing to do.

Ship work-in-progress code to production

if (featureFlags[SuperSecretFeature].isEnabled) {
  navigator.goTo(SuperSecretScreen())
} else {
  navigator.goTo(BoringOldScreen())
}

Merging code daily effectively translates to shipping work-in-progress code in production builds. To keep customers from running new features or incomplete refactorings of old code, we use feature flags.

While this may sound obvious to some, I’ve worked on teams that instead held off merging their work until it’s ready. We were able to make it work, but it had huge downsides. Feature branches isolate your team, preventing other developers from seeing what you’re working on until it’s done. In large teams, this often leads to people organizing merge-conflict resolving parties. They’re not fun.

Feature flags are also great for undoing mistakes. At Cash App’s scale, refactoring code that’s used by millions of customers is sometimes scary, but I feel comfortable knowing that I can push a button and remotely switch all our customers from BottomSheetV2 to BottomSheetV1 if an obscure bug is found in v2.

JUnit snapshot tests

com.squareup.cash.mooncake.AlertDialogViewTest > message only FAILED
java.lang.AssertionError: Images differ (by 0.7%)

While unit tests are great for increasing confidence in making changes to existing business logic, the same hasn’t been true for UI. Companies often rely on a combination of Android tests and manual QA, both of which aren’t ideal. Android tests are super slow and manual testing is… well humans will make mistakes. We’re solving this with paparazzi that lets us write snapshot tests as JUnit tests, without the requirement of an actual device.

Automated snapshot tests are great for ensuring that our layouts look pixel perfect with various configuration factors like display density, dark mode, up-scaled font size for accessibility, etc. Paparazzi is being actively developed, but you can try out an early preview: github.com/cashapp/paparazzi.

---

Wrapping up, I like to think that when an engineering team is empowered to merge fast, it both demonstrates and cultivates trust in each other. Barriers to merging code are best used for high-level design discussions and pain points in the codebase, and not as a substitute for pairing sessions.