preface

I have read several code Review articles recently:

Google engineer Michael Lynch

  • Mtlynch. IO/human – code -…
  • Mtlynch. IO/human – code -…
  • The last two Chinese translation analysis

How to make code Review better

  • Stackoverflow. Blog / 2019/09/30 /…

After reading it, I was deeply impressed. I found that the company I worked for did not attach enough importance to code review, and it was often regarded as a trouble.

Any software is co-developed, so code Review is very important. It can help you reduce code quality problems, improve development efficiency, improve stability, and at the same time ensure the stability of the software architecture, preventing the code structure from being malevolent and difficult to maintain.

Abstract

What is Code Review?

First of all, code review is an activity, which can be called “code review”, from the simple reading of a friend’s code by one person to the reading and analysis of the code by the team in the office.

Role division

From the figure above, we can see that there are two types of characters:

  • Reviewer – A person with whom to read the code
  • Author – The person who writes the code

Coverage of Code Review

  • A good code review examines code correctness, test coverage, functional changes, compliance with code specifications and best practices, and can point out obvious improvements such as hard-to-read writing, missing variables, boundary issues, large numbers of commits that need to be broken, and so on.

  • A good code review examines the need to introduce code, whether it fits into existing systems, whether it is maintainable, and whether the code is consistent with existing system logic in an abstract way.

How to determine whether merge is possible

  • A good code review will easily pass those open PR, at least until it is fully discussed, but every reviewer needs to give feedback on the part they care about after the review, whether it’s “looks good” or using the abbreviated word “LGTM”, and then follow up clearly. For example, collaborative software notifies authors for further feedback.
  • A good code review will be more flexible in practice, leaving suggestions for improvements for some urgent changes, but passing quickly, allowing the author to resolve the remaining issues through subsequent code submission.

Use code style specifications to resolve code style disputes

Arguing about code style is a huge waste of time in code review. Consistent code style is important, of course, but code Review time shouldn’t be spent discussing where parentheses go. Your best bet is to avoid this argument by maintaining a code style specification.

conclusion

In general, both sides should be humble and learn in the process of code review. In the process of review, they should try to avoid deadlock and conflict. Once a conflict occurs, the leader must quickly find a balance point. Harmonious mutual help and friendly team relationship is more important.

Combined with the previous summary:

Part1 – A good code review should do the following

  • More comprehensive, from correctness to system impact assessment.
  • Pay attention to tone, from giving constructive awareness to perspective-taking.
  • Complete the review in a timely manner, from full discussion to improvisation.
  • Enhance communication, from face-to-face communication to flexible choice of the most efficient communication methods.
  • Prioritize, from tagging to automating with engineering tools.
  • Be nicer to new people.
  • Avoid collaboration across time zones and use video conferencing when necessary.

Part2 – Good implementation of code review

  • When initiating a code review, the code submitter should provide a list of changes
  • After code review is completed, end with LGTM (looks good to me)
  • Instead of reviewing style formatting problems, these can be solved using the Lint tool. Team specifications can be changed, and give strong reasons.
  • Review requests should be handled immediately upon receipt. If you are too busy to handle them, transfer them to others immediately
  • Don’t ask too many questions, no more than 20-50
  • Give a good code example when asking questions
  • Don’t say “you” in comments. You could say “we,” or you could say code without a subject, or you could say interrogative instead of imperative
  • Comments should be based on code “principles” or specifications, not your own opinion

Part3 – Good implementation of code review

  • Note 5 above, because it is theoretically possible to upgrade the code from F to A, but it would cost the author and Reviewer too much effort and time, and may cause unpleasurable reactions in the first and second rounds. The way to do it is, he goes from F to C, and next time he goes from C to B or even A
  • Note the same problem 2-3 times, the rest tell the author to fix other similar problems
  • Code that is not in the scope of change and has problems should not be included in this review. If you have a handy naming problem or are extremely easy to fix, you can mention it as optional
  • Reviewing multiple 300-line changes is a better experience than reviewing a single 600-line change
  • When you see good things in your code that benefit you, praise them appropriately
  • If the rest are variable naming and spelling errors, and optional changes, you can simply approve without too much fuss. Not procrastinating is the point;
  • What if there is an impasse between author and Reviewer? Give in as soon as possible, or ask your manager for help. The longer the standoff, the more damaging it is to a relationship that is far more important than code quality. Don’t try to fix everything at once.