Skip to content
loading...

How to make code reviews developer-conflict free?

twitter logo github logo ・1 min read  

Code reviews seem to be a blocker on our team ever since. It is big, it takes time and it slows our delivery. Personally, I am having difficulty giving feedbacks and comments. I am torn on enforcing code standards and suggesting practices, and reducing frustrations for the developer. We are , most of the time, fast-paced so the developer usually have another task to work on. I feel like I am being the blocker by giving too many feedbacks when the developer should have moved on to another task.

How do you do it with your team? Quality of the code or quality of the human code? Where is the balance?

twitter logo DISCUSS (6)
markdown guide
 

Nurturing a healthy culture around code reviews takes effort and maintenance. Some things I’ve found helpful:
a) Always use inclusive language (“we” or “us” instead of “you”)
b) Speak about the code, not the individual
c) Discuss why code review is valuable, not just from a code quality standpoint, but also as a key way in which teammates can learn from one another
d) Decide not to nitpick. If something is a personal preference and is a nit, don’t say it.
e) Automate away any parts of the process you can. Decision fatigue is a real thing and it saves everyone energy if the linter in CI catches an error so that the reviewer doesn’t have to think about it. Side benefit is that if anyone is annoyed by having an error pointed out, it’s directed at the robot rather than a team member :)
f) To this end, pay attention to the kinds of things you’re commenting about in reviews. Could this be enforced with a static analysis or formatting tool? Add that rule to your linting setup and never talk about it again! Is this a nitpick that really is only about personal preference and has no bearing on correctness, readability, or performance? Let it go!

Ultimately I think it comes down to building a culture of trust and respect, and getting everyone on the same page about why code review is important and what benefits you all get from it.

 

I think coding standards should probably fall outside the scope of code reviews. I would be inclined to do the following:

  • Have everyone discuss and agree a consistent coding standard for each language you use, ideally an industry standard one like PEP8 or PSR2/12
  • Set up linting and code quality tools on your projects to enforce those standards, and where possible automate correcting them (Things like ESLint and PHP CodeSniffer will do this), and provide feedback in editors and IDEs
  • Consider setting up continuous integration to catch these issues and report them

That way, no-one has an excuse. You've all agreed to abide by the coding standard, and after the initial on-boarding everyone will start to appreciate the benefits of consistent code styles. If someone isn't co-operating you can say "Bob, we all agreed to abide by these coding standards", and that should be the end of the matter. You can also then start to add other automated tools to the mix, such as static analysis, copy-paste detection, and so on.

The great benefit about automated tools for these kinds of things is that there's no judgement - it's just presenting these issues factually so it's less likely someone's ego will get in the way.

 

If your team strives for excellence, you should enforce (or suggest at least) best code review practices. I even had written a team code review guide once for one of the companies that I worked for, with main rules and links to code style guides and stuff. When you have a single source of truth like that for reviewing practices, there's less space for arguing and fights and hesitation. Also, you can actually prevent many hard cases by providing proper automated or semi-automated tools. It can be linters, code formatters (combined with pre-commit hooks), decent CI (which checks build preventing merging if there's any problem), and PR templates with necessary lists of code quality checkboxes for self-review prior to submitting a PR.

 

That depends on your goals but if you feel like you're a blocker then:

Is it readable?
Does it do what it set out to do?

These are the the only things which really matter when reviewing code.

Enforcing biases only slow down the process.

If you have code standards which aren't being followed, question why that is. You may be able to automate that process.

If you find that PRs are overwhelming, try breaking them down into smaller more achievable tasks. This will help dev and review.

 

+1 to automating the enforcement of coding standards. These should never be a reason to hold up code approval: either automate them before code review, or allow the review to pass and fix coding standards afterwards.

One thing which might seem strange but is worth trying: increase the frequency of the code reviews which should reduce the size of the code being reviewed each time. Infrequent code reviews are more likely to be based on very large amounts of code and the review process can increase even further as the reviewers have to understand much larger amounts of code.

 

Code Reviews are never a blocker, but are extremely important to spread learning and understanding of the codebase/standards. Said that, many times, code reviews can became a sort of “chat”, where elements are added / removed continuously...in that case, impacting productivity.
Personally, coming also from another industry, I would spend more time on planing and communicating ahead of each assigned task...in order to align and clarify the outcome/result.

Classic DEV Post from Mar 25

What are some good questions to ask when you're inheriting a codebase?

When you are getting a project assigned that has a considerable codebase already, what are some good...

camilla profile image
I love refactoring.