#1
❓ But I need these code reviews before the code is merged! How could I ensure quality without them?
❗️ Although the generally good and necessary practice, code reviews turned into their opposite in the past decade or so.
Primarily thanks to GitFlow and likes, teams have been adopting asynchronous in-process code reviews as an industry standard and the best practice.
📚 Thanks to them, Git and code reviews fell victim to Semantic Inversion and Semantic Diffusion so otherwise very good tools, both of those turn into pebbles in the shoes of most of the modern teams.
First and foremost, asynchronous code reviews prevent teams from continuously delivering the results of their work, preventing them in turn to get quick and timely feedback.
However, it is just the obvious consequence. More dangerous are the effects it has on team dynamics and team effectiveness.
The idea behind creating Git in the first place was to facilitate an open-source contribution to Linux kernel development.
That is a low-trust environment, where contributors are not trusted by default, and their contributions have to be reviewed before integrating into the codebase.
If you consider your team to be a low-trust environment, you have much bigger issues.
Effective teams are high-trust environments, so asynchronous code reviews come as a handicap and a pain to them.
The usual excuses include (but are not limited to):
➡️ “We need to review the code written by juniors before integrating”
➡️ “Reviewing code before merging ensures quality”
📚 Both of those are clear signs of dysfunctions, described in my earlier posts in detail – How to Integrate a Junior Team Member and Indications that Your Team Might Be Something Else.
💡 Asynchronous code reviews block, stall, or obstruct your development process in other ways.
💡 Although it somehow got to prominence and became a counterproductive industry standard, GitFlow is not suitable for teams practicing Continuous Delivery.
#2
❗️ Asynchronous code reviews, serving as a gated phase in your process (usually on pull/merge requests), are costly and often hurt the quality of the results.
💡 Run a simple experiment – ask a question about the value you are getting from the code review phase in your process.
If there are any findings at all, the usual answers are:
1️⃣ We are catching errors from junior members before merging the code.
2️⃣ We are getting a better solution when somebody else takes a look before merging the code.
3️⃣ We catch code style and naming inconsistencies.
Both 1️⃣ and 2️⃣ are costly and ineffective ways of getting the desired results.
❗️ There are much better ways to introduce and grow a junior team member to the team (1️⃣). One effective way to do so is described above.
❗️ Proposing solutions (by “taking another look,” i.e., 2️⃣) AFTER the costly implementation had been done and expensive decisions had already been made is irresponsible, expensive, and ineffective. There are much better ways to do so.
❗️ Having manual code style checks and compliance with code standards is both naive and ineffective. It is much more effective to do so using tools (e.g., linting, static code analysis, and such), so 3️⃣ doesn’t make sense either.
💡 Unless you have a very good reason to have such a phase in your process, e.g., working on an open-source project or working in a toxic low-trust environment, find a way to remove the asynchronous code review phase from your process.
❗️ It is expensive, ineffective, and irresponsible!
💡 Additionally, it does not bring the value you are telling yourself it does.
#3
❓ If the recommendation is to avoid doing code reviews asynchronously, in-process, before the merge, when should I do them? Should I abandon code reviews altogether?
💡 Firstly, code review is a useful, valuable practice, and (in general) should not be abandoned.
🎨 Artists take a step back from time to time and take a look at their work to make sure that everything fits well and “sits right.” However, they do not do that after each brush stroke. Sometimes they do, but mostly it is usually done after the part of interest has been done. They take a step back and do a quick review.
🖥️ In the same manner developers and development teams should take a step back from time to time, and invest some effort into making sure that everything “sits right.”
Over time, working on a product and developing a software solution we gain new knowledge and refine existing one. Our perspective inevitably shifts.
💡 So, what looked nice a few weeks or months ago, might not be that nice when we look at it from a newly gained perspective.
❗️ But beware! If we do it daily before each merge, we build tolerance and fall prey to “boiled frog” syndrome. We stop seeing the forest for the trees.
💡 Instead of obstructing the development process using a low-value asynchronous code review phase, a way to do it better and put more value into it is via conveniently frequent, possibly irregular code review sessions.
💡 Instead of reviewing the code in isolation by yourselves and writing comments about your findings in lengthy message threads, do it together as a team. Collaborate and contribute in real time!