🥢 👈🏾 ⚪️ Code review - improving the process 🧛🏻 🚣🏽 👋🏻

Imagine a team where Code review is not conducted. Developers write code, and without checking make all the changes to the main branch. After a while, the functionality expands or bugs are found, they return to the source code, and there all the variables are named with one letter, there is no following the architecture, and the quality is not the best. This poor-quality code will accumulate and one day the moment will come when, with any little innovation, the project will begin to fall apart: in the best case, the development time will increase, in the worst, the project support will become impossible. And this despite the fact that once upon a time the task was completed and everything worked well.

How can this be avoided?The answer to the question in the title is Code review.

Code review is a code check for errors that improves the stability and quality of the code.

Now imagine a team where Code review is already underway, but in the process the developers swear among themselves, Pull request is not approved for a long time, tasks begin to freeze. The process from the beginning of the task to its appearance in the project is stretched, and with it all work is slowed down.

Pull request / Merge request is a request to the project team (person or group of people) for approval and application of changes to the selected branch. As soon as the Pull request is created, a discussion of the written code takes place before approval. Changes may be suggested during the discussion. After approval, the current changes fall into the selected branch.

Listed below are recommendations to help speed up Code review and improve its quality.

We divide the question into three parts and consider each separately:

Technical part: how to check the code and what to look for when checking?
Communication part: how to prevent disputes and reduce stress during the discussion?
Organizational part: what can be done to make the Code review process efficient?

Part 1. Checking the code

Run the author code at home

Run the code on your own and see how the changes work in conjunction with the rest of the code. This helps to find problem areas that are not visible in the web-based interface. Try to see the code comprehensively, avoid focusing only on local changes. So you will quickly figure out the code and quickly find architectural inaccuracies, if any.

Remember user experience

Pay attention to how changes in the code will affect the end user. Even the most beautifully written code can be useless for users, which will lead to additional tasks, bugs, and can also strike a blow at the reputation of the company and the product.

We look at the general logic

Developers can successfully solve their problem, but break the work of other pieces of code. To prevent this from happening, look not only at how a specific task is being solved, but also at how changes will affect the work of other services, modules and the entire project as a whole.

We look at the code architecture

The architecture of the code determines how much time in the future we will spend on expanding, adding functionality or editing a bug. Also, the architecture of the code can affect the potential appearance of bugs in case of changes in the project. Ideally, expanding and adding new functionality should not lead to refactoring.

We write easier

Pay attention to the code re-complication: the simpler the code, the easier it is to read and easier to maintain. Get rid of complex pieces of code.

Multithreading

If the project implies work in several threads, then see what happens if there is a delay during the execution of the code in one of the threads, and how such cases are worked out.

Excessive optimization

As classic Donald Knuth wrote, premature optimization is the root of all evil. It’s better to optimize only what is needed here and now.

We work out errors

Pay attention to how the project will behave if it was not possible to execute a line of code, a block of code or a request to the server. Often, developers interrupt the execution of a function without error output, but such cases need to be worked out.

Compliance

The code must comply with the agreement, the code style. The uniformity of the code is not a whim, but a necessity: such a code is easier to maintain, and it is easier to understand this code.

Names and appearance

Remember other programmers who will have to understand your code. Readable code simplifies its further support. Names must be understandable and accurately describe the class, object, function, etc.

Code Comments

Comments should answer the question: “why is this done?”, And not “how is this done?”. Remember this.

Part 2. We discuss

The main rule of discussion: any comment should be aimed at the code, and not at the person who wrote it. It works in the opposite direction - do not take comments as criticism of you personally. The task of code review is to make your code better, because what you have not noticed may be noticed by others. Colleagues can propose alternative solutions, and this is the potential for professional growth. It is important to remember that the discussion of the code is not a contest in wit or a show of “who knows more”, therefore sarcasm, hidden aggression and rudeness are inappropriate in it.

As a rule, pull request is carried out on special web hosting services (github.com, bitbucket.org, gitlab.com, etc.), where it is possible to view the changes and leave a comment on a specific piece of code.

We comply with the agreement

Again, the code must comply with the agreement. However, if such an agreement does not exist, you should not ask a colleague to add a space or indent in the code.
In controversial moments, you can agree with the whole team right in the code review process, but asking to follow these rules is better at the following code review, so it will be easier for everyone to accept them. By the way, a documented guideline in writing style removes almost all disputes.

We offer an alternative

Avoid statements like “you did wrong ...”, “why, why do you write like that?” and others. They are perceived as criticism and lead to excuses. It is better to write a comment about the content of the code, without resorting to the identity of the author. Also try to avoid orders and coercion: people do not like when someone orders them, and perceive such comments negatively.

You can proceed as follows:

We write what is wrong with the code
Why is it better not to write
We offer alternative options

We remain within the framework of the problem

Often you can see comments on the code that was before and did not touch. No need to comment on what is not relevant to the task. Any third-party edits can take a lot of time, and can be perceived negatively, so it’s better to look at how a person completed the current task, and not ask him to refactor the project.

Praise

If you see an interesting or cool solution, feel free to give praise. In addition, it is a great motivation for your colleagues to continue to write good code in the future.

All comments are equal.

It often happens that one programmer technically knows more than the other, as evidenced by the gradation of junior, middle, senior, team lead. It is not worth highlighting the comments of one group as more important. This devalues the opinion of some developers, which can lead to indifference and formal participation in code review. When everyone's opinions are equally important, code review is more productive.

Clearly express your thoughts

For productive communication, write as detailed as possible and explain every detail. Everyone has a different level of knowledge, and so far no one has learned to read minds.

Asking questions

Feel free to ask your colleagues why their proposed option is better than your current one. This is a great opportunity to learn something new and grow professionally.

We resolve conflicts

It happens that a person does not accept all the arguments and cannot offer his own, refuses to do something. A few practical tips for this case:

Most conflict situations can be resolved by speaking in person, in a friendly tone.
You can concede, since you cannot write code while you are at war.
You can attract the whole team and decide together how best to write the code.
In the current code review, leave everything as it is, and make the controversial issues separate tasks for refactoring, tasks, because the words “then fix”, as a rule, never come to life.

Part 3. Improving the process

We describe what they did

We describe the essence of the task in the pull request header (or make a separate comment) and what steps have been taken to complete it.

Divide the pull request into parts

A large chunk will be watched for a long time, discussed for a long time and corrected for a long time. Divide the code into small logical parts - this way the process will go much faster.

Reply to all comments

It is advisable to respond to each comment so that the team does not have any ambiguities. Other developers should understand that you read their comment, did the necessary work, and made corrections. Constantly opening a pull request and looking at what has been fixed and what is not is very inconvenient and time-consuming.

Search all?

There are different approaches - to look for the maximum out of the possible or first to comment on important architectural moments, and after correction, pay attention to the little things.
Both methods have the right to life. I believe that the second option is more time-consuming: imagine that after the correction you need to fully review the code again, comment and wait for the corrections again. It’s much faster to carefully go through the code once, leave comments and then check the corrections.
If there are architectural inaccuracies and it is clear that minor errors themselves will disappear after fixing the architecture, you should not waste time commenting on details in this section of the code.

Waiting for everyone?

You can not wait until everyone approves the pull request and agree that the approval of 80% of the reviewers is enough to close the task. This will speed up code getting into the main branch, which is undoubtedly more beneficial for business processes, but can lead to disagreement in the team and indifference to code review.

The second option is to wait for the approval of all involved developers. The quality of the code will increase, but the process itself will slow down significantly. The choice in favor of speed or quality, each team should make their own.

Little things

If there are no serious comments on the code, then you do not need to wait until all small inaccuracies are removed. They can be indicated in the comments and immediately approve the pull request - the author of the code will feel calmer, his loyalty to the team will increase, he will feel that he is trusted. And, of course, the speed of pull request will increase.

Conclusion

Code review is an important part of the development process, which affects the project as a whole, so it is dangerous to ignore it. Some of these recommendations will help speed up code review, some of them will make it better. I hope my experience and knowledge will be useful to readers of this article.

Code review - improving the process