Slowed down by code reviews

Question

Our dev team have agreed that all code contributed to the project must be reviewed. We branch off master and merge back for every unit of work (sometimes a whole feature, sometimes just work-in-progress that passes tests). However, as I'm becoming more productive I'm finding myself increasingly slowed down by having to wait (sometimes days) for code to be reviewed, then waiting for another review if there are suggested changes.

I'm working around it by moving on to new features while waiting for a review, but this means context switching, working off old code and more time spent merging branches. Our team is also small so I don't want to stress out or make my co-workers look bad by stacking up endless code reviews. I'm also at risk of making myself look bad by having large amounts of work in-progress/in-review.

They're also usually fairly small changes, so sometimes finding myself pushed to cram more features into a branch, but this feels like a bad idea?

I've not had much experience working as a team, so wondering what steps I can take to improve the productivity of myself and/or the entire team, or even encourage other developers to be more proactive in doing reviews? Would also be interesting to hear suggestions from people who have been on the other side of this.

Answer 1

It seems that you are already quite productive with your work, as you say you are improving your development time and also are the one waiting for others to review you code so you can continue.

Also, I would not worry about having tasks on review making you look bad. Quite contrary, those pending reviews mean you are making good progress with your development tasks, and given the situation is not like you can rush or do the reviews all by yourself.

I will go even further and say that you should not be concerned about you doing your job making your coworkers look bad. Although it is noble for you to worry about your coworkers, it is their problem if they are taking too long to review the work you have done.

It may be you have few experience working as a team, but it seems you productivity is doing ok. Also, have in mind that it's probably not your job/responsibility to worry about team productivity and efficient reviews; that is something your lead or manager should be doing, so don't be tormented about having to make your team shine. In this case, setting an example (as you mentioned in a deleted comment) by being efficient in your reviews is a great way to encourage them to be more productive

Another important thing you mentioned (but also deleted the comment) is that Code Reviewing enables you to share knowledge with your team. The bugs you may find with the reviews are nothing compared to the insights, best practices, and knowledge domain your team gets when doing them. This will be more valuable on the long run that fixing some bugs a few days earlier.

Speaking of long runs, it also seems your team is just recently implementing this review strategy; surely when you get more used doing it your review productivity will improve significantly. It is expected of any learning curve to be harder at the beginning.

As mentioned in comments, if your team is including a new task that all team members must do (thus requiring time from your part) you should also balance your job burden accordingly, so you don't get overworked.

As a last suggestion, you could make better use of your time waiting for the reviews on your code by doing documentation, start to plan future tasks or do some other things that you can take out of the way in the meantime. Hope this helps, wish you luck.

Answer 2

If you find yourself regularly waiting hours for a code review, this is normal, in my experience. You're doing the right thing by moving onto other tasks in the meantime, even if you have to do some annoying merges. As other answers have noted, smaller batches of code are more likely to get reviewed quickly (and the reviews are likely to be of better quality, as well).

If you find yourself regularly waiting days for a code review, then that starts to turn into a major productivity issue, especially if everyone else on the team also has to wait days for a review. Since you say this is a relatively new strategy for your team, I would suggest talking to your manager about how to encourage quicker turnaround. Maybe there could be a quick meeting where good review practices are discussed, both for the reviewer and the author. Occasionally my manager at work has to remind our team not to keep others waiting forever for code reviews, as well.

Answer 3

We had this problem at a previous company I worked at. Every single code change was required to go through a Pull Request (PR) and code review with at least two approvals from experts in the domain (usually our own team members sufficed but certain people's approvals were weighted more heavily than others) before it could be merged into master. Here are some things we found helped speed up the review process:

• Try to have as few changes as possible in each PR to make them quicker to review.
• Make the PR description as detailed as possible with regards to what you're changing and why. Any UI changes should have before and after screenshots. This gives the reviewers context about the change so they can more easily identify things that may not be intended.
• New PRs are posted to our team Slack channel to increase visibility. If working in another team's domain, the PR is posted in their Slack channel as well.
• If working in another team's domain, solicit help from that team in doing that work to begin with so the PR isn't a surprise. You're also much less likely to have done things in a way that they'd rather you didn't.
• Each team member spends 15-30 minutes each morning and afternoon reviewing any outstanding PRs. We usually did this first thing in the morning and when we got back from lunch so we were less likely to forget.
• If a PR has gone more than 24 hours without being reviewed, verbally remind the team.

Between all of those, we found a nice balance between writing and reviewing code, and thus were able to deliver code at a pretty consistent rate.

Answer 4

I agree with the other post (that you shouldn't be overly concerned about this, and that it reflects well on you). But I would also add, that I can think of two ways you could maybe reduce the amount of "code review work" for your co-workers:

Make sure every change is as well tested as possible

Specifically, if you are getting stuck in a cycle of:

1. New Feature
2. Code Review
3. Fix 1 to New Feature
4. Code Review
5. Fix 2 to New Feature
6. Code Review

etc, etc.

then better testing will help you to optimise. Think about your ratio of new features to fixes to bugs (especially bugs that could have been found earlier in the cycle).

Minimise the amount of new / changed code per new feature

In other words, don't do in ten lines what you can do in two. This is good practice in any case, but it also makes life way easier for your colleagues.

Also, don't randomly refactor or reformat code, unless it is really necessary.

Answer 5

Try packaging your work and making it easier to review. Estimate the time it will take to review the items.

If you have a lot of small independent items, package them into sets of code into logical chunks that can be reviewed within a specific period - like one hour. Schedule a meeting and send links to the repo items. Talk to the team and explain you are trying to clear the backlog through a series of short and targeted meetings.

From the other side, you can put everything together into a growing, single review request. But this is sort of pushing in the direction you don't seem to want to go. Every time you complete a new unit of code, add it to this single request, and update the total time. It may be that people just aren't thinking about the backlog growth. If you manager sees a code review request and it says, "This is a code review request for item X committed on {x/x/x} | {21 days ago}, item y committed on {x/x/x} | {20 days ago}, item... It will take approximately 40 hours to review these 200 items", it will drive home the point.

Answer 6

Our dev team have agreed that all code contributed to the project must be reviewed.

This is the crux of the problem, and given the organization size, it might be unavoidable. Your team needs to reevaluate this decision based on current outcome (choking productivity) because it's harder to read code than to write it. Based on that, the decision to code review everything is more than doubling everyone's workload and you need to factor that as a cost in your team's decision.

The way we do it in our (smaller) shop is:

Phase 1: First 3 months (probationary period), everything is reviewed. The developer learns how we do things, how the system is built, how to find existing code they should reuse (as opposed to rewrite / duplicate), naming conventions, formatting, etc. We gauge the developer's ability / talent. After this period the dev is either retained or let go.

Assuming the developer is retained, we switch to phase two:

Phase 2: Once the developer understands the way we code in a particular codebase, they self review. The developer has the option to request a review at any time, but they are not mandatory. They go back and look at their commit and make sure nothing is wonky. We trust the developer and they are accountable to be diligent in this step. Commits are available to anyone, so a manager or lead can continue to monitor commits or sample them. This cuts the cost in half. If a developer isn't diligent in this step, they get a talking to.

I've observed that most large companies typically do things, especially development, in a very inefficient manner. They have their reasons, but a lot of it is inertia and timidity. I don't like working at large companies because of that.

Answer 7

Your goal is to create software that works, and that is known to work, at the least possible cost (and cost is usually the employees that get paid, and hopefully never the cost of angry customers). If your productivity goes down to 20% due to code reviews, it looks like something is wrong.

Your steps before the code review should be: Write code that compiles. Put it in a state where you would pass it if you had to review it (except for mistakes which happen). Test it so you can say “it works”, unit tests may help, but it must work with your testing. (Assume QA is better at testing than you are.) Merge your current main line into your branch so there won’t be merge conflicts. And then you put it up for code review.

Then discuss the role and powers of the reviewer. You should assume that author and reviewer are equally capable. So the review should find errors that need fixing at the second earliest point in time. Personal opinions of author and reviewer should not come into it. Different people have different styles. Having the author adapt the code to the reviewers style, and doing the opposite on some code where the roles are exchanged, that is a waste of time. And important: Don’t review anything outside the change. If you are the reviewer and find something you don’t like near a change, take a note, suggest it as s change to the team or product manager, but keep it out of the review.

And once you follow this - why do you need two reviewers? One reviewer will be helpful. How much does the second reviewer help at all?

Some tactical things. Never mix refactoring and new work within one pull request. Refactoring is easy to review. Refactoring plus behaviour changes is a nightmare to review. Moving things should be done separately. If two authors make changes in the same file and both append new items at the end you’ll have merge conflicts; add new code where it belongs not at the end.

And if you have to wait for a review: Once you’re at s point where a code review usually doesn’t require many changes, branch not from the main line but from the unreviewed branch. So you can continue working without wait, with minimal merging.

Answer 8

Things that I would like to add from myself:

• Remember that when someone spends too much time in code review may also mean that he gets it difficult to read through the code. I suppose, that you practice clean-coding. If not, make sure to write readable code using meaningful naming conventions and satisfactory code structure. Avoid tricky-hacky oneliners that can deceive your teammates.

• Also teach your co-workers that if they don't clearly understand a line, it is a valid concern and should be mentioned as a comment in review.

• Spend the extra time for helping others do their task more efficiently. Perform some pair-programming with your colleagues. Help them resolve issues that are slowing them down. You will see, that they eventually improve after some time.

• Teach others, how to perform good code review. In my company, we usually add more than two people to a single review to allow them to learn the art of good review from themselves. In previous job, I noticed something like pair-code-review: More than two people were discussing face to face about concerns in someone else's code.

• You can also spend some time to improve automation in your development process. If there are still some parts that require developers to do manually, think about writing scripts for it. It may be automatic deployment, continuous integration, box test environment. Someone mentioned about static analyzers, but I suggest to use them rather as an improvement, not to replace code reviews.

• You can also spend more time to increase code coverage by tests.

Edit

• Just another thing came into my mind: You can also use spare time to prepare a code review checklist. It is very useful mind tool to go through code review very quickly and not missing anything. It could help your colleagues a lot.

Answer 9

So I'm going to take the contrarian point of view here. I've worked with [REDACTED LARGE SOFTWARE COMPANY] for 6 years, and after going through the ramp-up phase, found my productivity plateauing at roughly 20% of what it was before (and after), due to mandatory code reviews occurring in different timezones.

The long-term productivity argument is, to put it mildly, nonsense. Sure, at the beginning of the learning curve you do get some insight on how things are done around here, and you learn to put your pride aside and get your brain reformatted in the llama. But after a year or two? You know the llama well enough, thank you very much, and much of the review comments start coming across as misguided if not outright ignorant. And yes, they become a drag on your productivity and sense of usefulness at work.

The coping strategies I found are:

• If you have a say in whom reviews your code, use that power wisely. Getting someone to trust you so that your code reviews get rubberstamped promptly is good; scratching someone's itch (whatever it might be) to the point that they will assign Perforce OWNERShip of the code to you and leave your hands free to implement whatever evil plans you have, is better.
• Use Git and private branches and stay as far ahead of the code reviewer(s) as you can comfortably manage. If your shop doesn't do Git, invest whatever amount of time it takes to install / build a gateway between the in-house VCS and Git.
• Deal with it and plan for released code lagging days or weeks behind your Git tip, wasted time rebasing on top of the changes of other people with better connections or EQ as yourself, etc etc.

Answer 10

While the other answers are mostly correct, one detail is worth noting: it is the exact purpose^(*) of the code reviews to slow down implementation. As you mentioned yourself, sometimes "there are suggested changes".

Without the slowing down (the code reviews), things would lead to a mess, and ultimately to potential project / company death.

I have the feeling that you are too concentrated on doing what is fun (coding) and you are not actually involved enough in reviewing other people's work - which is not nice. Not only that you do not contribute to the review process, but you rebuke the people contributing (to the review process), while doing their programming work in parallel.

If you will review more of the code of your colleagues, while they review yours, you will not be able to get so much ahead of your colleagues with coding, and things will balance themselves. But if you continue to write code while they review code, then you are the cause of the imbalance.

---

^(*) As a matter of fact, the slowing down is a side effect, considering that nobody actually wants to slow down coding just for the purpose of slowing down. But the slowing down is tightly woven into the code review process - and most of the slowing down cannot be mitigated. (Tnx to @JoeStrazzere for pointing out my incorrect wording)

Answer 11

It's nice that you're producing code and are productive. That said, it's not really useful until the code is merged.

In my opinion, you should work on producing better reviews. Make it as easy as possible for reviewers. Write a good description. Organize the commits. Make sure your code is well tested and readable.

By making your code as easy as possible to review, you'll soon see your code being reviewed quicker. From my point of view, this is a sign that your PRs are not as good as they could be.

Simply saying "I'm so productive, they can't keep up" is great for your ego. I wonder if there's another side of people that simply don't like reviewing your code, which is why it piles up.

If you make concise, well documented and tested code changes, with the code respecting the code culture and generally being of high quality, and you're open and eager to adapt it to other people's feedback, to make it easy to maintain, you will usually find people do make a bit of time to go through your PR.

If it's a long PR, without particular care taken to make it easy to review, it will often linger.

I can't answer what's what here. But being complacent is rarely a good thing.

Answer 12

While this question is tagged Software Industry, I think it would best to step back a bit and look at it from an industry neutral point of view.

Your companies process for the work you do involves an external quality review step. Your productivity is thus measured not by the time it takes for your task to go from start to finish, it is measured by the time spent between start and ready for review (in total, if more work is required to pass the quality control review, then that extra work gets added on to your total).

So your personal productivity is not at issue, this external review does not slow you down.

Now there are two caveats to be aware of, one that is directly your responsibility, and one from a company efficiency point of view.
1. First the personal, you obviously want to have as little rework as possible. Some is expected, possibly even desirable, a lot is a problem.
2. Secondly, and this is more industry specific, you want to make it as easy as possible for your work to be reviewed. If the QA takes longer because you didn't take the extra steps necessary to ensure that testing was easy and clear, that can be laid at your feet, particularly if your collegues do take steps to help the testers. So making it easily testable can be considered part of making it ready for review.