Code Review Behavior — Let’s be professional
Code review is the process of peer verification in the software development process. Often, the code review may be seen from different perspective to different people. In this blog, I will give you some ideas about how to face the problems amongst engineers in order to help business go ahead without “people” conflicts. We cover code review not only from the engineering but also from the business perspective!
The objective is to bring more self-awareness and knowledge which can be helpful while reviewing the code. For example, how to deal with professional vs personal conflicts; promoting empathetic behavior instead of personal preferences.
i.e. Behavioral awareness + tech expertise = Amazing code quality
So, what is the Code Review about?
The code review is referred to as the process of reviewing the software code. There are collaborative code review tools e.g. Github, Gerrit, GitLab etc. Using these tools, the author of the code names the reviewers who can asynchronously review online.
Some questions for you think about it:
- What is Code Review for you?
- What do you look for while reviewing code?
- Would you review your own code in the same way as compared to when you do someone else’s? If it is the same, that’s great! You act as a professional. If it is not the same, well, I believe you must have strong reasons for that.
Code Review in an Engineering perspective
Normally some common review points in the process are security issues, performance, algorithm, code formatting, test coverage, functional requirements etc. However, sometimes, the non-functional requirement of the code is ignored i.e. scalability, maintainability, readability etc.
What do you look for while reviewing someone else’s code?
- The author’s experience?
- Do you take into consideration if the colleague is Junior / Middle / Senior?
If you have something to add, always suggest and explain your approach, why it’s important or better, no matter how experienced the author is.
- Business requirements?
More detail in the below section “Code Review in a business perspective” i.e. functional and non-functional requirements.
- Company workflow?
Reviewing and requesting to fix a small thing NOW leads to new commit / push…. running all the Continuous Integration (CI) pipeline again.
It can be painful depending on the company / team, taking hours (github resources, running tests, …).
How you say it matters!
Let’s consider a trivial example of code with no new line in the end of file:
Reviewer1: “I always add a new line in the end of the file, so add a new line on all the files in this commit!”
Author: “Ah, what a small thing. no one told me that. I don’t like this reviewer. I’m not going to deploy it today as I am upset.”
Reviewer2: “A minor comment here — could you please add a new line at the end of the file. It will meet our coding guideline.”
Author: “Sure! It is a quick change, will do it right away. Thanks for pointing out.”
Which kind of review comment would you like to have?
Prefer Standard Practices over Personal Preferences
In order to gain the best out of the review process — when you submit your comments, consider the following questions before showing the disagreements:
- Does it follow the COMPANY standard?
- Is this an OFFICIAL programming LANGUAGE standard?
- Is this a MARKET standard?
- Or just a PERSONAL preference? (…and the reviewer doesn’t approve it while it is not changed due to personal preference)
If you are suffering such a stressful situation, how can you handle it? You should take a pause and ask the above questions. It may help avoid the stupid genius disease. “Stupid genius” is a person who always says: “… What I do is the best, it works better. Do what I said!”.
Code Review in a business perspective
It should (or not) be a concern for code reviewing? It is quite important in my opinion!
- What if the Code that you are reviewing DOESN’T COVER 100% requirements?
privileges / access on a resource (local database / external api, …)
code belongs to another department/company and requires licensing?
and so on
- What if it COVERS 100% of the business requirements but it doesn’t make sense in a business perspective or it has lack of requirements definition? For example, the software calculates a price without validation if it’s underpriced or overpriced.
How critical is it?
Considering that you work in big company, how bad can it be if we focus only in Engineering and discard Business problems just because it “was not written on the issue” and in the end something is underpriced?
We cannot ignore it! The business needs to work as it should be.
For non-critical things, like code improvements, we can approve it and deliver the feature for Business now without forgetting our Engineering world problems: Tech Debts
Apart from the engineering excellence, we need to consider the business requirements as well.
Conclusion
- Be professional, not a stupid genius
- Don’t let a technical conflict become a personal conflict, blocking business
- Don’t be a selfish person, think about the company and be a team player
Credit
I want to thanks Ahmed Hagag, João Nadais, Mohammed El Hakim and Shivendra Tiwari for the great contribution on this article.
