8 points to make judges fall in love with you

Introduction to external objective factors and only thinks about how to make the review efficient and meaningful from the perspective of a code implementer and a reviewer. In other words: how to make the reviewer fall in love with me (the reviewee).

As we all know, every team with technical pursuits tries to make Code Review ultimate. As the saying goes-unreviewed code is not worth mentioning. Why Code Review is so important, I won’t go into details in this article, just make a brief summary:

• Find problems early, prevent trouble before they happen
• Quickly improve the programming ability of new classmates in the team

However, the dream is very full, the reality is very skinny, the team’s review activity and review quality are often worrying, for various reasons, from technical TL, down to the implementer of the code function, and even the product manager (well, it must be the business is too heavy. Leading to no time to do Code Review) can be the culprit for not doing Code Review well. Today, as a developer, Xiaobian puts aside the external objective factors, and only thinks about how to make the review efficient and meaningful from the perspective of a code implementer and a reviewer. In other words: how to make the reviewer fall in love with me (the reviewee).

Why let the judges fall in love with me

In the discussion of previous articles, we tend to focus more on how to improve the level of reviewers, how to find out the problems of reviewing code, or how to enable the reviewee to quickly solve the problems raised. But we ignore the feelings of the reviewer, it seems that the reviewer is a BUG-finding machine without emotion. Of course, some automated scanning tools (such as Codeup's defect inspection, vulnerability analysis, etc.) are not excluded, they are indeed AI robots. But in addition to this, many other members of the team are still required to intervene as reviewers due to issues such as business understanding of the code and the coding style of the team. Let’s leave aside the perfunctory review. It takes time for reviewers to participate in a review. It takes time to read, understand, and point out questions. Therefore, we should let go of our precautions, take the review process calmly, and be grateful to the reviewers.

How to make the judges fall in love with me

Now that we have made up our minds to have a sweet Code Review journey with the reviewers, how can we make the reviewers fall in love with me? I analyze from the whole cycle of a review, which are as follows:

• [Before review] How to prepare before review;
• [Under review] How to deal with discussions/problems during review;
• [After review] What needs to be done after review.

submission for review

Focus:

• Take a close look at your own code
• Commit
• Guaranteed test baseline

Carefully examine your own code

In a review, the most taboo thing is to make some low-level mistakes. This will not only give the reviewer a bad impression, but also waste the reviewer's time on some low-level mistakes. Therefore, we must carefully review our own code before submitting it.

format problem

Formatting problems often occur in some newly established teams. Everyone has different development habits and coding styles. In single-person development, there is no problem. When this project involves multi-person collaboration, it becomes very troublesome. For example, in the following review, there is no actual change but the formatting of the coding style. But the dense file changes often make people uneasy.
Therefore, before submitting, we should set up our editor formatting tools according to the team's specifications, and check whether there are formatting problems before submitting, so as to avoid such problems from wasting everyone's time in Code Review.

misspelled

Spelling errors are low-level errors. The current mainstream editors support spell checking. As long as you take the editor's highlighted warning seriously, you can find out.

Code specification scan

Some very basic problems can be scanned out by automated tools. For example, for Java development protocol detection, vulnerability detection and security scanning provided by the code platform. After submitting, I see that all the commits changed on the branch are green passes, and my mood will be much better. After all, the code we provided to the judges is also a rematch player after the primary selection.

Commit (Commit)

When it comes to commit, many students think it's nothing. Isn't it git commit ? Or when committing, because Git's mandatory requirement must include a message, just enter a few submission instructions that only you know, and then git push pulled down. For example, the following submission instructions have been seen in the code repositories of many brother teams.

I won’t go into details about this kind of submission instructions. Anyone who has taken over old projects and filled in holes in old projects knows it. Therefore, in the review stage, we must complete the submission.

• Key point 1: Split submission by function, reject the big Diff review

In normal development, everyone often develops multiple tasks in parallel. Many students don't like to switch the changed branches back and forth. As a result, many reviews are submitted by multiple functions, so the reviews naturally become very large. Or when developing a relatively large requirement, the function failed to be split and refined, resulting in all the changes in the same review, even in the same Code Review. E.g:

In this review, dozens of feature submissions are mixed, and reviewers are easily lost in the logical maze of different features. What's more, it will submit tens of thousands of lines to be added or deleted for review. For this kind of super large review, I don't believe that the reviewers will read it carefully. According to the statistics of self-media sharing, modern people's attention can only be focused on 5 minutes. If a sharing exceeds 5 minutes and cannot be finished, people will often give up because they can't remember the previous content. The same is true for reviews. Tens of thousands of lines of changes are all in the same review. Even if the function of rivet read files is provided on the review page, it is difficult to understand the logic of the context in a short time. Therefore, the reviewer needs to spend more than half an hour of independent time to read the review changes, and this is very crashing for normal developers. Therefore, when we complete a function, we should split it by function reasonably and make the submission smaller (small is beaut iful). When the changes are too large, you should consider splitting multiple reviews.

• Key point 2: Describe clearly the content of this submission, the changes, and the impact of the changes

The beginning of a review, from opening the page to the beginning of the review, the general order of the eyes is: review title -> review description -> change list -> change details

The title of the review and the description of the review are often generated by the content we submit. As the first two key points at the beginning of the review, the review submission description is very important. For example, the following example:

I believe that you, as reviewers, want to turn off the review after reading this review (some irritable guys even just refused). Puzzling review title, blank review description. Even if we sit in front of the reviewer's computer with Code Review, or nail the review explanation, this kind of experience is very bad, because everyone does not want to be distracted. Just like when we write code, everyone is more willing to focus on the editor during the development process, rather than switching back and forth between different screens (checking needs, being interrupted by people, etc.). Therefore, we should write a good description, and try to make everyone can complete all the work of the review on one screen (on one page) when the review starts.

In addition, a good submission description can allow the reviewer to perceive the changes and the impact of the changes in advance. For example, the following:

From the description, the reviewer can determine the place of our change and the impact of the change, so that the reviewer can enter the state faster, without having to read the detailed content and guess what we have changed.

• Key point 3: Functional changes and non-functional changes should be separated as far as possible

In a function, in addition to functional changes, we may also include some non-functional changes. If they are all kneaded together, it often looks very uncomfortable. For example, the following situation:

In this change, in addition to some functional changes, I also revised the document and formatted the previous formatting issues. Although they are all the same changes, the look and feel is still very poor. Just like a teacher's lecture, a good teacher must summarize and teach according to content, rather than mixing algebraic geometry.

Therefore, we can make further splits in the review submission. Still for the above example. Our adjusted look is:

Click Submit all on the left side of the review page, and select the submitted content that we need to review independently through the submission information.

Select the submission of functional changes and review the functional changes.

If you choose non-functional submission, you can only focus on non-functional changes. For example, here we only need to look at the changes in the README alone, and the whole process is refreshing.

Guaranteed test baseline

In many already formed projects, there are often sufficient test coverage scans. When we submit the code, the first thing we have to ensure in terms of function is that the previous tests can pass (unless this test case has been abandoned or there are errors, we should only carry a function to fix it), this is also Respect for reviewers, because reviews that cannot even guarantee the correctness of functions are meaningless.

In the cloud effect, we can use the target branch of the review as the protection branch, and configure the automated inspection in the automated inspection. Through flexible configuration in the assembly line, we need to check the stuck points in our project.

Then, when we submit the review, it will automatically trigger the card point we configured. If the card point fails, the review is not allowed. Therefore, when we submit for review, we must pay attention to the content of the card points and make our review "green".

review

Focus:

• Treat review feedback positively
• Code is the best explanation
• Forgive the reviewer for misunderstandings/errors
• Ensure the timeliness of Code Review

Actively treat review feedback

The reason why some students dislike code reviews is that they think the reviewers deliberately embarrass themselves. It cannot be ruled out that such problems may indeed exist in some more complex social environments. However, as a reviewer, we still have to take a positive look at the review feedback. After all, the premise of picking the bones in the egg is to have bones, and the question must be reasonable. The vast majority of technical people treat the code neutrally, and cannot actively face the review. What is more is that we are worried that our code is not good enough, and we are afraid that others will point out problems. But from another perspective, the reviewers are actually helping us. It is better to find problems in the review stage earlier than if there are problems online. In addition, in the continuous review, we can also quickly improve our coding ability.

code is the best explanation

When reviewers have questions about our code, we should not only explain, but also consider why such questions arise. Sometimes, although I gave detailed responses to the reviewer’s questions, the reviewer seemed to understand it. But is this review really over? Marx said that we should look at the essence through the phenomenon. After all, there may be more than one reviewer, even if other current reviewers see this explanation, they can understand it, but we cannot guarantee that someone will understand the ambiguity when the subsequent code changes involving this piece of code are reviewed again. Therefore, this kind of ambiguous review questioning, its own problem is that the code fails to do a good explanation.

Faced with this situation, it is generally recommended to add sufficient code comments. But I recommend that we directly refactor the code so that the code itself can explain its meaning clearly.

Forgive the reviewer's misunderstanding/error

Confucius said: No one can fail without a sage. The reviewers will also have misunderstandings about the correct code during the review. At this time, we cannot be unreasonable and should remain humble. As mentioned in the opening chapter, the reviewers actually help us, helping us eliminate hidden dangers we don't know ourselves. Therefore, we should not rashly counterattack, but should respond rationally. While explaining, it is also a good review of our own code. In addition, we should also be aware of another problem here- exists that is reasonable . If the reviewer raises questions, does it prove to be unreasonable? Maybe the code is ambiguous and needs to be refactored?

Guarantee the timeliness of Code Review

The review should also be time-sensitive. Because the code reviewed by reviewers is often not the business of their own participation, the review cycle is too long and may not be remembered. Therefore, in the review, a reminder hook is often set. Cooperate with Dingding webhook to receive comments from reviewers as soon as possible.

Give feedback in time after the modification is over. This allows the reviewers to continue the review faster when the review has not been "cooled down", and everyone can leave work early.

after review

After the review, the content of the review will be saved on the platform, and the review can be viewed at any time later. In addition, the most important thing after the review is to choose a merger method. Generally, I will choose the squash mode to merge. Combine all the submissions in the review into one commit. So merged into the trunk, a commit is a function. Note that this is not the same as the previous submission split. In the submission review stage, we split the submission into functional changes and non-functional changes in order to facilitate the review. But in the merger phase because it has passed the review. We want to put a feature change into a commit.

After clicking Merge, multiple submissions in the review will eventually be compressed into one commit and merged into the target branch. This also guarantees the principle of one commit per function. It is very convenient to troubleshoot problems and roll back online.

The PS merger method should be decided according to the respective team's choice of project management and development mode. The above is just a simple example.

summary

To sum up, if the reviewer fell in love with me as a review, I would do this:

1. Carefully review your code before submitting, and reject low-level errors
2. Split the submission by function, and write the submission description
3. Let all unilateral and collective tests turn green and pass to ensure the quality baseline
4. Treat review feedback positively
5. Use code to explain the reviewer’s questions
6. Treat misunderstandings/errors of reviewers correctly
7. Correction/feedback in time to ensure the timeliness of the review
8. Choose the right way to merge after review

Author of this article: Chen Bojun, Alibaba Cloud technical expert, Git open source project contributor, responsible for the underlying architecture design and operation and maintenance development of the Alibaba economy code platform and cloud efficiency code platform.

---

Long press to recognize the QR code in the picture above and sign up now

6 special sessions

27 domestic and overseas technical experts

Invite you to share a feast of performance

Locked on June 23-June 24

9:00-12:00  14:00-18:00

We look forward to working with you to build a bright future for R&D efficiency!

Copyright statement: content of this article is contributed spontaneously by Alibaba Cloud real-name registered users, and the copyright belongs to the original author. The Alibaba Cloud Developer Community does not own its copyright and does not assume corresponding legal responsibilities. For specific rules, please refer to the "Alibaba Cloud Developer Community User Service Agreement" and the "Alibaba Cloud Developer Community Intellectual Property Protection Guidelines". If you find suspected plagiarism in this community, fill in the infringement complaint form to report it. Once verified, the community will immediately delete the suspected infringing content.