The Problem
I had an experience with a company I worked with years ago. Their code-review process was broken. It took weeks or more to get a review completed.
It was all done in the name of writing "good code," but the result was less than effective.
What I started to realize was that the process, for this company had become a way that the developers could push back against a restrictive environment.
Details
As a developer, we could be writing extremely problematic code and finding coworkers irksome for simply pointing that out.
At the other end of the spectrum, the code could be fine but a toxic culture in the organization creates endless nitpicking. In environments like this, the code review becomes an adversarial activity, many times not about code quality.
What should our goals be?
- Determine what is causing the stress?
- Determine what should the desired outcome be?
Code review processes vary so widely from team to team, so let's examine what the goals should be ...
The Code Review
Considering the subject of code review itself (the code), there are two conceptual levels at play.
A code review has important human ramifications. People learn and share knowledge through the review process. And they use it to prevent the kind of defects that result in frustration for the team. Developers get emotionally invested in their creations (we own the code we write).
A good code review process helps with code quality and helps developers learn. It is the most effective defect prevention method. This put a good code review ahead of even automated unit tests.
Often, the closest a developer gets to actual organizational power comes during interviews and code reviews. Decent human beings handle this well, looking to mentor and teach. Unfortunately, this power can also be abused.
Check and Improve
The first strategy for dealing with a distressing code review is the simplest.
- Is the review genuinely helpful (if poorly delivered)?
- And, is there room for improvement?
This may be a case where a developer can, "grin and bear it." Work, being a four-letter word, will result in exposure to some annoying people. Sometimes, they just happen to be reviewing code.
Take the feedback seriously. Internalizing it. And see what happens. As developers, we get better at writing software. The hope is that these reviews become less annoying over time.
Ask For Change
If the "distressing" reviews continue, there comes a time when we need to create an inflection point. Be direct and ask for different behavior, avoiding the objectionable things. If their behavior changes, the problem is solved, even if there was some stress getting there.
If not, you have learned something and have information to act upon. I personally think life is too short to spend dealing with unpleasant people.
Don’t Involve Management
It’s tempting to find a referee. The idea is that they will intervene and referee.
Engineer Less Interaction
Do not involve management directly about interpersonal differences. This does not mean we cannot involve them indirectly. This method can take some time, so figure out how to manage until this change is implemented.
Figure out how to work on something else or be somewhere else. Get creative at getting distance from the code reviewer.
You can certainly involve management in these discussions. In fact, you’ll have to. Don’t cite anything negative as your motivator ... leave that out. Instead, offer to work on a project that others don’t want, or ask to transfer because of your interest in your new, requested project.
Most of the employees couldn't directly approach the reviewer, couldn't changed teams, and could not go over their head to speak with management.
They left the company. I could clearly see that this was one factor of many that attributed to the high turnover rate.
The Solution
With the company I described at the beginning of the article, getting code reviewed was a "distressing" process.
In my case, I approached the reviewers that were causing the issues and had deeper theoretical discussions on code. This process created a connection that allowed me to smooth out parts of the review process.
It wasn't a straight-forward solution and I wouldn't always recommend this approach, but it did work at the time.
Top comments (1)
Good code reviews can be effective if there's a "wise architect" that can help guide the team (or several of them).
I know this sounds a bit hierarchical but often times you have people that are new in a project and people who have been there for a while. You also have different levels of skill and experience.
When new on a project, I always want to hear what others say about my code because it's a big learning moment about the architectural decisions that predate me.
However, as time goes by some bad practices can come to light and when the group talks about how to tackle them the code reviews can then help remind people to take care of them in the future.
I agree that nitpicking can be a waste of time. Nobody is ever going to write code nicely but some ideas like:
That last one is the most important in my mind. I've reviewed code where the approach taken simply will never work. No sense in even testing it. Make sure the right algorithms are used, that memory is used wisely, that there aren't any glaring bad hacks.
It does help if there's a more senior person setting the tone of things or otherwise the project can go in all directions.