How Lyft Does Code Review

How Lyft Does Code Review

images/Lyft_PressKit_08.jpg

With its liberal use of pink, signature moustaches and Undercover Lyft video series, Lyft has always branded itself as the fun-loving and friendly rideshare service. That brand extends through its company culture all the way down to internal developer culture and how they do code review.

“Emoji-driven development,” an internal practice of using emojis as commands to manage pull requests (more on this below), sounds crazy to outsiders but makes total sense in Lyft-world. Per Lyft software engineer Yann Ramin and former director of engineering Jill Wetzler, it’s the company value of “Make it happen” that encourages engineers to “take on big projects, propose new features, fix persistent bugs, and take risks.”

We talked with Evan Madow, senior software engineer at Lyft, and he shared with us how Lyft is structured for giving engineers the autonomy to make it happen. He explained to us what “emoji-driven development” actually is.

🚗 Strict process for tier zero services, autonomy for the rest

Lyft is organized into services that are tiered according to criticality. Tier zero services include Lyft’s ETA service, which tells riders the time until pickup, its mapping service, which provides accurate mapping for driver routes, and any service that will ruin the core Lyft experience if it goes down.

Tier zero services have rigid code review and deployment processes. They have processes around code ownership, which require that all changes go through domain experts who understand the core systems holistically. Owners’ files specify the five or six experts, one of whose approval must be sought for any pull request on a tier zero service. Code reviewers are mandated to look for the following:

  • A clear description of the change. This includes specific requirements that vary based on the service at issue.
    • Jira ticket links, other relevant PR links, and links to any experiments that this code involves
  • Tests
    • Unit tests
    • Integration tests
    • End-to-end tests (in many cases)
  • Code
    • Adherence to code guidelines that are specific to the service
    • The change is clean and doesn’t mix refactoring with new features
    • No copy/pasted code (keep it DRY—“Don’t repeat yourself”)
    • The change should be reasonably small
  • All review comments are addressed and resolved

For everything else—all non-tier zero services—individual teams are empowered to set their own processes and culture around code review build on top of some basic defaults:

  • Code review happens via the pull request model in GitHub. Developers work off of feature branches. When they’re ready for review, they push and open a pull request.
  • A change needs at least one +1 by someone on the team to have approval for merging into master.
  • All PRs go through a continuous integration process that kicks off automated testing in the form of unit tests, integration tests for upstream and downstream services with dependencies, and linting.

“As you get higher in the tier of the service that you’re working with, the code review process gets more rigid,” Evan told us. He likes this approach because there are classes of services and deployments, like “low impact web stuff and display code where keeping it frictionless is really important.”

Based on this, various teams can have “very different philosophies and very different ways of approaching code review,” Evan told us. That minimizes top-down control and allows each team to define what would work best for them to get stuff done.

Evan is on the Fleet Platform team, which manages all Lyft-owned vehicles, while supporting other areas of the business like Bikes & Scooters and Autonomous. We talked with Evan about how his team does code review, as an example of how an individual team might define its own process.

🚲 How the fleet platform team defined their code review process

Because the fleet platform isn’t on, what they call at Lyft, the “golden path,” it isn’t a tier zero service. Lyft’s golden path is “getting customers from point A to point B while providing the most delightful experience throughout.” If the fleet platform goes down for a few minutes, the core user experience will still be intact.

The upshot is that, in contrast to tier zero services, the fleet platform doesn’t have a top-down, formal process. Instead, their team has the autonomy to define their own code review culture and, rather than mandate a strict process, they’ve chosen to articulate and follow a few basic principles:

  • Keep PRs as small as possible.

A lot of times, when we work on larger product features, we’ll make a feature branch and cut units of work into smaller PRs. For the inevitable time when you have a very large PR where there are a lot changes involving various dependencies, and the line count gets out of control, we employ interactive rebasing to create smaller requests. It makes things a little bit more digestible and gets things split up into logical, functionally related PRs.

  • Get eyeballs on PRs to keep the team in the loop.

For my front-end code changes, I often tag backend engineers just for context, even folks who are not actually on this project but are on my team. This ensures they’re in the loop with larger architectural concerns. In those cases, we don’t usually expect comments or +1s. But just having more eyeballs on PRs is important to our team.

  • Keep the whole process lightweight.

We don’t have a very formal process. We don’t have templates for what our PR should look like in terms of composing a verbose description or ensuring X, Y, and Z are done. For product launches and large new features, we’ll have a rollout checklist just to make sure all of our ducks are in a row and there’s a rollback strategy if things don’t work out. Those will be specified in the PR description, which will link out to any relevant documents.

This approach to code review that gives the developer a few basic principles to follow rather than a specific process suits Evan well because, as he told us, “I don’t love process.” For example, he told us, “I’m not a fan of having really verbose checklists with predefined placeholder text in a new PR where you have to check off all these items and fill out all these different fields, because it can get very wasteful.”

Because code review is defined locally by the teams implementing the process, engineers like Evan can get tightly aligned with it and make it work for their needs.

🚀 Why Lyft’s code review comments are filled with emojis

When it comes time to merge, “No one actually uses the big green merge button,” according to Evan. That’s because they have emoji-driven development—they use emojis in the comments of a pull request to issue commands to drive the process.

Instead of pressing the green button, they write :rocket: or 🚀 to merge. They use :rocket: :pray: or 🚀🙏 to merge and skip policy checks (only to be used in case of emergencies). Reviewers write :+1: or 👍 to approve a pull request.

Emoji Alias Slash Command Action
🚀 :rocket: /merge Merge
🚀🙏 :rocket: :pray: /merge-skip-checks Merge skip checks
🔨 :hammer: /retest Rerun test jobs
👍 :+1: /approve Approve PR
😭 :sob: /revert Revert
👀 :eyes: /ptal "Please take a look”—request a review

The internal tool that powers this process is called Submit Queue, and it runs on comments on the main pull request page (not from commit comments or inline comments in PR diffs). Each emoji also has an associated slash command, which can be used to eliminate ambiguity and get explicit as needed.

“Getting on board with [emoji-driven development] and learning what all of the emojis do is a bit strange. But once you start doing it, then it gets really fun,” Evan said. “Your codebase has all of these cat emojis and rocket ships in it. It’s just very, very strange.”

What stands out to Evan is that the engineering culture at Lyft is one where a few engineers could have a fun and slightly weird idea about incorporating emojis into code review and then make it happen. And that has evolved into a defining aspect of engineering culture at Lyft.

Conclusion

Lyft hasn’t been content to imitate software development practices from Silicon Valley giants like Google. It has infused its own personality into its code review process, which has resulted in a distinct culture that works for them.

Whereas Google, for example, requires language readability review as part of every code review, Lyft only requires a strict process for critical services and provides open-ended defaults for all other services. While Google has a monorepo and a concept of code ownership around every change, Lyft has a polyrepo and attempts to make non-critical changes more lightweight.

Each comes with trade-offs, but what you end up with, in Lyft’s case, is a culture that values autonomy, moving quickly, and making it happen.


About PullRequest

PullRequest is a platform for code review, built for teams of all sizes. We have the world's largest network of on-demand reviewers, backed by best-in-class automation tools. Because code quality is important.

Learn more about PullRequest

Lyal Avery headshot
by Lyal Avery

February 2, 2020