Brief introduction:Today Xiaobian, as a developer, puts aside external objective factors and only thinks about how to make the review efficient and meaningful from the perspective of a code implementer and a person being reviewed. In other words: how to make the reviewer fall in love with me.
As we all know, every team with technical aspirations tries to make Code Review as good as it can be. The truth is — unexamined code is not worth mentioning. In this article, I won’t go into more details about why Code Review is so important, but I’ll just summarize:
- Identify problems early to prevent breakdowns
- Quickly improve the programming ability of new students in the team
However, the dream is full, the reality is very thin, the Review activity and Review quality of the team are often worrying, for various reasons, from the technology TL, down to the implementors of Code functions, and even the product manager (well, it must be that the business is too heavy to do Code Review) can be the culprit of poor Code Review. Today Xiaobian, as a developer, puts aside external objective factors and only thinks about how to make the review efficient and meaningful from the perspective of a code implementer and a person being reviewed. In other words: how to make the reviewer fall in love with me.
Why should the judges fall in love with me
In previous articles, we’ve tended to focus more on how to improve the reviewers, how to identify problems with the code being reviewed, or how to get the reviewers to quickly fix the problems they raise. But we ignore the feelings of the reviewer, as if the reviewer is an emotionless bug-finding machine. Of course, there are some automated scanning tools (such as CodeUp’s bug check, vulnerability analysis, etc.) that are indeed AI bots. However, in many cases, due to the business understanding of the code, the coding style of the team and other issues, other members of the team still need to intervene as reviewers. Leaving aside the perfunctory review of the mind, it takes time and energy for a reviewer to participate in a review. It takes time to read, understand, and point out questions. So let your guard down, be comfortable with the review process, and be grateful to the reviewers.
How do I get the judges to love me
Now that we’ve made up our minds to have a sweet Code Review tour with reviewers, how can we make them love me? I analyze it from the whole cycle of a review as follows:
- [Before the review] How to prepare for the review;
- [During the review] How to handle the discussion/issues during the review;
- [After the review] what needs to be done after the review.
Before submission for review
Key points:
- Take a hard look at your code
- Make a Commit
- Guarantee test baseline
Take a hard look at your code
In the review, the most taboo point is to make some elementary mistakes, which will not only give the reviewers a bad impression, but also waste the reviewers’ time in some elementary mistakes. So look at your code before you commit it.
Format problem
Formatting problems often occur in some fledgling teams, where people have different development habits and coding styles. In solo development, there is no problem. When the project involves multiple people working together, it can be troublesome. For example, in the review below, there is no actual change but the formatting of the coding style. But dense document changes, often can make people uneasy. Therefore, before submission, we should set up our own editor formatting tools in accordance with the team’s specifications, and check whether there are formatting problems before submission, so as to avoid wasting everyone’s time during Code Review.
Spelling mistakes
Spelling error is a simple error. Most editors support spell-checking, but if you are careful to check the warning highlighted in the editor, you can find it.
Code specification scan
Some very basic problems can be scanned by automated tools. For example, development protocol detection for Java, vulnerability detection and security scanning provided by code platform, etc. It’s much easier to see a green pass on the commit after the branch changes. After all, the code we presented to the reviewers was a finalist after a primary.
Make a Commit
When it comes to committing, many students think nothing of it. Git Commit. Or, when it’s time to commit, since Git’s mandatory to include messages, just type in some instructions that only you can understand and then Git push it. For example, I’ve seen the following commit notes in many of my sibling teams’ code repositories.
I will not dwell on this issue of submitting a note. Anyone who has taken over old projects and filled holes in old ones knows it. So in the review stage, we have to do a good job of submission.
- Point 1: Split submissions by function and reject large Diff reviews
In the usual development, we are often multi-task parallel development. Many students do not like to switch back and forth the branches of changes, resulting in a lot of reviews are submitted by multiple functions, so reviews naturally become large. Or when developing a large requirement, the functionality is not broken down and all changes are under the same Review, or even in the same Code Review. Such as:
With dozens of feature submissions mixed in, the reviewer can easily get lost in the logical maze of different features. In some cases, tens of thousands of lines will be added or deleted for review. I don’t have much faith that the reviewers will actually read the whole thing. According to the statistics of research on we-media sharing, modern people can only focus their attention for 5 minutes. If a share lasts longer than 5 minutes and cannot be finished, people tend to give up because they cannot remember the previous content. The same is true for reviews, where tens of thousands of lines of changes are made in the same review. Even if the review page provides the function of rivet the read document, it is difficult to understand the logic of the context in a short time. As a result, reviewers can spend up to half an hour of independent time reading review changes, which can be very frustrating for normal developers. Therefore, when we complete a function, we should reasonably split it according to the function, making the submission small (beaut iful). Consider splitting multiple reviews when changes are too large.
- Point 2: Describe clearly the content of the submission, the point of change, and the impact of the change
The beginning of a review, from the opening of the page to the start 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 start of the review, the review submission description is very important. Take the following example:
I’m sure all of you, as judges, are already thinking about shutting down the judges after watching this one (some grumpy guys even just refused). Convoluted review title, blank review description. Even if we sit down with a Code Review in front of a reviewer’s computer, or attach a Review explanation to a pin, it’s a very bad experience because people don’t want to be distracted. Just like when we write code, we prefer to focus on the editor during development rather than flipping back and forth between screens (looking at requirements, being interrupted, etc.). Therefore, we should write a description so that at the beginning of the review, everyone can do all the work on one screen (one page).
In addition, good submission notes can make the reviewer aware of the change points and impact in advance. Take the following:
Reviewers can identify our changes and their impact from the description, allowing them to jump into the game faster without having to read the details and guess what we changed.
- Point 3: Separate functional and non-functional changes as much as possible
In a function, in addition to the functional changes, we may also include some non-functional changes, if all mixed together, often look very uncomfortable. Take the following situation:
In this change, I in addition to do some functional changes, but also incidentally modified the document, the previous formatting issues have been formatted. It’s the same change, but it still looks very bad. Just like a teacher lecturing, a good teacher must summarize and teach according to the content, instead of mixing algebra and geometry.
So we can do a further split in the review submission. Again, for the example above. The appearance after adjustment is as follows:
Click “All Submissions” on the left side of the review page, and select the submissions we need to review independently through the submission information.
Select the submission of functional changes and review the functional changes.
Selecting a non-functional commit allows you to focus only on non-functional changes. For example, here we just have to look at the changes in the README alone, and the whole process is refreshing.
Guarantee test baseline
In many established projects, there is often an adequate test coverage scan. When we commit the code, we first have to guarantee is on the function of previous test can pass (unless there is this test case has been abandoned or mistakes, that we should carry a function to fix), which is respect for reviewers, because even there is no guarantee that the functional correctness of the review is meaningless.
In the cloud effect, we can configure the automated check in the automated check by using the target branch of the review as the protection branch. Through the flexible configuration in the pipeline to set our project needs to check the stuck point.
Then, when we submit the review, it will automatically trigger our configured card. If the card point does not pass, the review is not allowed. Therefore, we should pay attention to the content of the card points in the submission of the review and let our review “green” up.
In the review
Key points:
- Be positive about review feedback
- The code is the best explanation
- Forgive the reviewer for any misunderstanding/mistake
- Make sure Code reviews are timely
Be positive about review feedback
The reason why some students dislike code reviews is that they think reviewers are deliberately trying to make things difficult for them. It cannot be ruled out that such problems may indeed exist in some more complex social environments. However, as reviewers, we should take the feedback of the reviewers positively. After all, the premise of picking the bones is to have bones, the question must have its rationality. The vast majority of techies are neutral in their approach to code, unable to actively face reviews and more worried that our code is not good enough and that people will point out problems. But look at it another way, the reviewers are actually helping us find problems early in the review phase, rather than when things go wrong online. In addition, in the constant review, we can also quickly improve their coding ability.
The code is the best explanation
When a reviewer asks a question about our code, we should not only explain it, but also consider why the question arises. Sometimes reviewers seem to understand even though I respond to their questions in detail. But is this really the end of the review? Marx said, we should look through the phenomenon to see the essence. After all, there may be more than one reviewer, even if other current reviewers see this interpretation and understand it, but there is no guarantee that any of the subsequent code changes that cover this area will understand the ambiguity when they are reviewed again. The problem with ambiguous review questions, therefore, is that the code has not been well interpreted.
In this case, the general recommendation is to add adequate code comments. But I’d rather just refactor the code and let the code itself explain what it means.
Forgive the reviewer for any misunderstanding/mistake
Confucius said: To err is human. Reviewers also have misconceptions about correct code during reviews. At a time like this, we should not be indifferent to others. We should be humble. As mentioned in the opening paragraph, the reviewer is actually helping us to eliminate our own hidden dangers. So we shouldn’t rush back, we should respond rationally, and while explaining it is also a good review of our own code. In addition, we should also be aware of another issue here — existence is reasonable. If the reviewer raises a question, does it prove that there is an irrationality here, perhaps because the code is ambiguous and needs to be refactored?
Make sure Code reviews are timely
Reviews should also be timely. Because the code reviewed by the reviewers is often not the business they participate in, the review cycle is too long, and they may not remember it. Therefore, in the review, it is common to set a hook for reminders. Cooperate with the webhook reception of the nail, it can perceive the opinions of the reviewers as early as possible.
Provide feedback immediately after the revision is completed. In this way, the reviewers will be able to continue the review faster and leave work early when the review is still not “cool”.
After the review
After the review, the content of the review will be saved on the platform and can be reviewed at any time later. Beyond that, the most important thing after the review is to choose a merge approach. In general, I choose the squash mode for the merge. Merge all the commits in the review into a Commit. Merged into the trunk, a commit is a function. Note that this is not the same as the commit split above. During the submission review phase, we split the submission into functional changes and non-functional changes to facilitate the review. But in the merger phase because it has passed the review. We want to put a change to a function into a commit.
After clicking Merge, the multiple commits in the review will eventually be condensed into a Commit merged into the target branch. This ensures the principle of one commit per feature. It is very convenient to troubleshoot problems later and to roll back on the line.
The P.S. merger mode should be determined by the respective team’s choice of project management and development mode. The above is just a simple example.
conclusion
To sum up, here’s what I’d do if a reviewer loved doing it for me:
- Take a hard look at your code before you commit it and reject silly mistakes
- Split the submission by function and write the submission instructions
- Let all single side, set test green pass, ensure the quality baseline
- Be positive about review feedback
- Use code to explain the reviewer’s questions
- Deal with reviewers’ misconceptions/mistakes
- Timely correction/feedback to ensure the effectiveness of the review
- Select the right merge method after review
Author: Chen Bojun, Alibaba cloud technology expert, Git open source project contributor, responsible for Alibaba economy code platform and cloud code platform underlying architecture design and operation and maintenance development.
Long press the QR code above to register now
Six special
27 technology masters at home and abroad
Invite you to a potency feast
Lock June 23 – June 24
9-12:00 PM – 18:00
We look forward to working with you to build a better future of R&D efficiency!
Copyright Notice:The content of this article is contributed by Aliyun real-name registered users, and the copyright belongs to the original author. Aliyun developer community does not own the copyright and does not bear the corresponding legal liability. For specific rules, please refer to User Service Agreement of Alibaba Cloud Developer Community and Guidance on Intellectual Property Protection of Alibaba Cloud Developer Community. If you find any suspected plagiarism in the community, fill in the infringement complaint form to report, once verified, the community will immediately delete the suspected infringing content.