Push and Pull
kellanem.com> You should review PRs, you should review them in a timely fashion. You should, you should, you should. That’s all Push. Any wonder that so many teams struggling with PR dwell time?
The problem with this is nobody is performance managed on PR reviews. Features are often what get put on performance reviews, so it becomes beneficial to the PR author to go work on another feature than review someone else’s PR. Reviewing a PR for a feature doesn’t get your name attached to it. More often than not, your review is considered a formality imposed by upper management and impediment to the feature in the eyes of line managers. Those headwinds turn PRs into a classic Volunteer’s Dilemma [0].
I spend a disproportionate amount of time on PRs at $client and recently got dinged for not performing “enough” so I stopped reviewing PRs.
PRs have dwell times of weeks. It’s all broken, but somehow we are all convinced that this is better than a formalised change review process.
Never mind the inability to plan work, or having to have yet another discussion with some corporate random that happens to be reviewing your PR and has a thousand questions as to why this particular functionality needs to be implemented, and why weren’t they consulted - dude, here is all the process documentation, go complain at management, not me.
But in the meantime, management wants to know why x was not delivered on time.
Teamwork takes time. I think a lot of people do not understand that.
As soon as we get above two or three tightly knit people internal communication starts to become the bottleneck. It's inherently serialized. It's the human equivalent of Amdahl's law.
Never has it been more clear than when remote work took over in organization that are not built on it from the start. While we all feel more productive without office chit chat it all has to be compensated somehow if we're going to be aligned with common goals.
Perhaps we need to dedicate 25% work time to PR's. It's just half of what pair programming is and plenty of people are productive doing that.
eng-practices has a page about speedy reviews and says that reviews should happen in a day or less: https://google.github.io/eng-practices/review/reviewer/speed...
It's a pretty established practice within the Goog eng teams I've interacted with. Consistently and noticeably failing to meet the 1 day response would be minor grounds for perf mgmt, I reckon. Never would happen for just that problem by itself, but would be mentioned alongside other problems perhaps. Just providing an anecdotal counterpoint...
My company tracks PR reviews. They aren't tracked in source code (and to my knowledge source isn't used as one of the metrics).
The engineer can put forward that they spend their time on PR reviews and it's had X benefit.
So how do you quantify the difference your review makes? That cost/benefit analysis is hard to do objectively because the counterfactual outcome of deploying without review is not observed. You could model the various probabilities of failure and their expected cost, but it is arduous and hand wavy at best.
don’t companies like amazon track the number of PR comments as a metric?
Kanban would help, with max tasks in progress for team and PR being a task.
Now you can’t do a new thing you have to take the PR!
I don’t really understand the pull. It sounds like push is the shoulds and pull is the musts. As far as I understand it, push is the ‘do X’, pull is the ‘hold accountable when they don’t do X, or praise when they do’.
But these seem as two sides of the same coin, not as orthogonal concepts.
They’re not orthogonal and I don’t think they’re claimed to be. The biggest differences I see are about time and generality.
Push is a about something general and in the future. “Every week, put in a status update.” and “As PRs come in, review them as soon as you can.”
Pull is concrete and about something in the past. “I saw in your status update yesterday you’re blocked by the auth bug.” and “Let’s start the meeting by checking our PR queue.”
Phrasing it like that, it makes it obvious why pull is underserved. People in general, especially software engineers, love talking about the general future. In almost any software process, there’s a huge advantage in giving more attention to the concrete past then it gets right now.
The team that I’m on starts every weekly team meeting on Friday morning with “positive shoutouts”, where we just say good things other people did that week. Concrete, past, positive. Great way to start a meeting. (It is also a “low politics” team, so I have yet to see much gamesmanship.)
Those async status updates really hit home as an example. This was a good, quick one and something I'm going to put some attention towards in the future.