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?
Top comments (5)
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.
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.
I think coding standards should probably fall outside the scope of code reviews. I would be inclined to do the following:
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.
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.