loading...

How to make code reviews developer-conflict free?

devcamilla profile image camilla ・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?

Discussion

pic
Editor guide
Collapse
kaicataldo profile image
Kai Cataldo

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.

Collapse
ollimac73 profile image
Camillo

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.

Collapse
fyodorio profile image
Fyodor

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.

Collapse
matthewbdaly profile image
Matthew Daly

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.

Collapse
danspratling profile image
Dan Spratling

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.