The Anatomy of Slow Code Reviews
Posted by aviator_co@reddit | programming | View on Reddit | 9 comments
Almost every software developer complains about slow code reviews, but sometimes, it can be hard to understand what’s causing them
ysustistixitxtkxkycy@reddit
Solid summary, the only missing one I caught is "are code reviews used as a political tool"?
My favorite solution has long been "everyone checks in without code reviews for quick turnaround, but every change has to have been signed off on before reverse integration"
valarauca14@reddit
Still waiting for my Juniors to realize when I say; "Hey looks good, I'll do a full review & approve it Monday. I have other tasks that are bit higher priority right now."
On THURSDAY AFTERNOON is a purposeful leadership decision from somebody who is aware of the project status, team velocity, task management, and quarterly goals BUT MOST CRITICALLY cannot grant PTO requests.
pip25hu@reddit
Would not work by us I'm afraid. Here, after you submit a changeset for review, until the review response arrives you start working on another task.
bisen2@reddit
Yeah, I can't imagine submitting a PR for review and then being like, "Okay, I'll just sit around until it is reviewed." Even if we don't want to hop directly into the next big project, there are always maintenance tickets or other code reviews or project planning that needs to be done.
ysustistixitxtkxkycy@reddit
That is typically the expectation, although where I used to work the complexity of changes and time to build made this type of task switch incredibly costly.
Main-Drag-4975@reddit
Working more than one ticket at a time is almost always the wrong move. Keep looking for a reviewer. If no one’s free, help one of them finish their ticket. Then they review yours, then you’re both free.
ysustistixitxtkxkycy@reddit
Thank you for that, I've never seen them used as a positive political tool.
The worst case I've seen were massive delays caused by one reviewer who had maneuvered into a position to be required for signoff, followed months later about complaints about how the project was late.
deamon1266@reddit
Code Reviews are fairly easy to do if the responsibilities are clear.
What works well for us in enterprise context.
TL;DR People get paid to write software. An Engineer is not necessarily a Manager. Trust, Respect, Tests and a good Failure Culture makes Code Review blazingly fast and easy.
A contributer is responsible for delivering good enough quality software. He or she must actively stste if there is a closer look desired which most of the time is resolved in pairing.
The reviewer has supportive responsibilities and looks out for obvious mistakes, reading tests, and also may provide helpful insights with project specifics stuff like internal libraries, which may is not known to the contributer. A reviewer may approve a PR even if there are objections stated.
It is the contributers decision if, when or how the comments or objections are adressed.
This means in short, the reviewer will not take most responsibility for oversight or a decision to merge poor quality code. If the contributer makes a mistake, no worries, we will fix and learn together.
If the contributor cant be trusted, then we have a diffrent issue we can't solve in PR.
MayBeArtorias@reddit
From my point of view it’s always about the understanding of trust and responsibility.
To beginn with: if I can’t trust my teammates and/or I can’t take the responsibility of reviewing code, it resolves to a me problem. And what I mean by that =>
Imagine I work with a junior, I know his success and learning is directly related to my commitment of checking her/his code. If the task was clear and given junior can handle it alone, I will just check the PRs general structure and tests. Is there the possibility that given Junior made a mistake and I didn’t see it in the PR … of course there is and then we have to fix it (ideally together). But this is always the case, so I rather I accept.
If it’s a very big change, I rather do actual code reviews.
If it’s a non junior coworker I expect the person is doing her/his job, while keeping in mind that I’m not perfect either. That’s why I’m scanning the code for the “typical” errors and check the tests (if architecture is a thing, make tests for it as well). Or again it’s a big change, then we go through the code together.
This is why and how I always do quick reviews. The code won’t become better if I’m letting it stand for 5 days. It rather blocks everyone at the end and it is just burning money. And the money aspect is the key argument to bring to your PO/PM when you explain why reviewing code is always a high priority