What's up with code reviews?
I recently saw this tweet getting on my feed
One of the biggest cultural shifts in my experience from Mozilla to Stripe is code review speed. At Mozilla you'd often wait days and have to hunt down people to review PRs.
— James Long (@jlongster) June 3, 2021
At Stripe my PR is often reviewed within 10 minutes.
That makes a huge difference for shipping fast
It’s a really interesting sharing, because I’ve been on the other end of that tweet. Chasing people down, declaring being blocked by code reviews, and even seeing managers take it up a notch by adding “code review” as a state in the Jira tickets so they could track how long are tickets “stuck” in that state. Declaring, “Monday’s” are for code reviews and such other things.
I would love for code reviews to be as short and sweet as the ones in the tweet, but I think it has other implications. After merge how long will it take for it to reach production? What’s the point of having new features or fixes stuck for a long time in a branch?
One of reasons I’ve recently come to experience in this process is the lack of trust that are an inherent part of code reviews.
Hot take: in-house development has been influenced too much by the GitHub open source PR driven development process. A process driven by zero trust doesn’t fit well in a team with trust.
— Patricia Aas 🐢🏳️🌈 (@pati_gallardo) March 20, 2021
Why we need to review code of people we trust? Can we fix a potential bug in the next release? How fast would that be? Can style be changed later? Can we iterate to optimized options later? Is the missing test case relevant to the feature?
Are releases so painful or risky, that we want to minimize releases?
The more we automate, the less we review, in a way. But automating the chasing of people to review makes it worse.
The lack of trust, also has an effect on who is trusted. Meaning, that even though we reviewed the code by some team mates. If it’s not one of the “most” trusted, then it won’t get merged.
Do these trusted individuals need to review every change? Do they need to know everything? In the end, pull requests should produce code that is understood by everybody on the team. But does that mean that the code has to be reviewed by everybody on the team?
I’ve seen mention ensemble working (AKA mob programming) to attack this problem. Meaning that the whole team produces the code. That definitely works, from the perspective of more than one person reviewed and code the work, such that at the end it could be merged immediately.
But the incentives need to right, otherwise management will think they’re “sacrificing” developers. And you need to take into accounts people not being present, days off, sick days, or even checking twitter during the session. So then you need some threshold of people to work in a certain feature to consider it “approved”.
Where am I going with all of this? Not sure, really, I have more questions than answers at this point. What I do know, is that having a lot of WIP is definitely detrimental to a team’s velocity. And avoiding reaching such a place, where the WIP due to code reviews breaks the team cadence every sprint is really hurting your delivery of software.
Happy coding!
Oscar
Related
- 1586 Reading Other People's Code with Patricia Aas
- Those pesky pull request reviews
- Reviewing Pull Requests – Chelsea Troy
website | twitter | github | linkedin | mastodon | among others