Article From:https://www.cnblogs.com/immiao0319/p/9124543.html

(Clicking on the above public number can be quick to pay attention to.

 

Compiling: actuarial dog, English: Michael Lynch

 

Recently, I have been reading articles about the best example of code review. I notice that the focus of these articles is to find bug and ignore the other parts of code review. How to communicate with constructive and professional problems? Not related! As long as you identify all the bug, the rest will come to pass.

 

I can only assume that all the articles I have read come from the future, when all developers are robots. In that world, your teammates welcome criticism of their code without a deliberate wording, because handling such information can warm the hearts of their cold robots.

I want to make a bold assumption that you want to improve the code review in the current world, when your teammates are all human beings. I also have to make a more bold assumption that the positive relationship between you and your colleagues is a goal in itself, not just an adjustable variable to minimize the average cost of a defect. In these casesWhat will happen to your censorship practice?

In this article, I have discussed some skills, which regard code review as both a technological process and a social process.

What is code review?

“The term code review “can refer to a series of activities, from simply standing behind a teammate reading the code to the single line analysis of the 20 participants. I use this term to refer to a formal and written process, but it is not as important as a series of on-site code review meetings.

The participants in the code review includeauthoras well asExaminer:The author writes the code and sends the code to the censor. The examiner reads the code and decides when the code will be integrated into the team’s code base. A review can be completed by many censors, but I have made a simplifying assumption that you are the only examiner.

Before the code review starts, the author must create one.Change table。The author wants to incorporate source code into the team code library, which includes a series of source code changes.

When the author sends the change forms to the examiners, the review begins. Code review isloopIt happened. Each cycle is a complete roundtrip between the author and the censor: the author sends the change, and the examiner gives written feedback to the change. Each code review includes one or more cycles.

When the examinerApprovalThese changes, the review end. This usually refers to the shorthand of giving LGTM, “looks good to me”.

Why is this difficult?

If the programmer sends you a change list, they think the change list is excellent. You wrote them a detailed list explaining why the change list is not good. This is the information that needs to be handled carefully.

This is one reason why I don’t miss IT, because programmers are very lovable. For example, in the aviation industry, those who overestimate their skill level are dead.

Philip Greenspun,ArsDigita The co-founder is from Founders at Work.

The author easily interprets the criticism of his code as implying that they are not qualified programmers. Code review is an opportunity to share knowledge and make engineering decisions. But if the author regards the discussion as personal attack, this goal can not be reached.

In addition, you are also faced with the challenge of writing to convey ideas. The author can not hear your tone or your body language, so it is more important to convey your feedback clearly and carefully. For an unsuspecting writer, there is no offensive note, such as “you”.Forgot to close the file handle, it can be understood as “I can’t believe you forgot to close the file handle! You are a fool. “

Skill

  1. Let the computer do a boring part

  2. Debate with style guide

  3. Start the review right away

  4. From the high level, step down

  5. Generously use code examples

  6. Never say “you”

  7. Express the feedback as a request, not an instruction.

  8. Associate annotations with principles, not opinions.

Let the computer do a boring part

Under the interference of meetings and mail, there is very little time to focus on code. Your mental and perseverance is even more short. Reading the code of a teammate is a cognitive burden, requiring intense attention. Don’t waste these resources on tasks that computers can do, especially when computers can do better.

A blank error is a remarkable example. Compare the energy spent by a human examiner in finding a correction and correcting it with the author, and the energy spent using just one automatic typesetting tool.

The energy needed by the human examinerThe energy required for a typesetting tool
1.The examiner looks for blank errors and finds the correct indentation.

 

2.The examiner writes annotations and points out that the error is indented.

3.The examiners read the annotations again to ensure that the wording is clear and does not contain accusations.

4.Author read annotation

5.The author corrects the code indentation

6.The examiner verifying that the author has properly handled the annotation

No!

The right side is empty, because the author uses a code editor. Every time they click “save”, the code editor automatically specifies the blank format. In the worst case, the author sends out the code for review, and continuous integration solution reports that the space is wrong. The author does not need the scruples of the examinerNext, fix the problem.

Search for mechanical tasks that can be automatically solved in code reviews. The following are common examples:

taskAutomatic solution
Construction of validation codeContinuous integration methods such as Travis or CircleCI
It was confirmed that the automatic test was passed.Continuous integration methods such as Travis or CircleCI
Verify that the code blank is consistent with the team styleCode typesetting device, such as ClangFormat (C/C++ typesetting device) or gofmt gofmt (Go typesetting device)
Identifying unused inputs or variablesCode linter, for example, pyflakes Python (Python linter) or JSLint JSLint (JavaScript linter)

Automation enables you to make more meaningful contributions as a censor. When you can ignore an entire class of problems, such as the sort of input or the conventions of the name of the source file, you can focus on more interesting things, such as function errors or readability defects.

Automation can also bring benefits to the author. Automation enables the author to detect careless errors in a few seconds rather than a few hours. Instant feedback makes it easier to learn from mistakes, and the cost of correcting errors is also smaller, because there are related backgrounds in the author’s mind. Besides, if they have to hear their own stupid mistakes.For self esteem, it is easier to hear from a computer than to hear from you.

Join your team to add these automatic checks to the code review workflow (for example, pre-commit hooks in Git or Github webhooks). If the process requires the authors to run these checks manually, you willMost of the benefits are lost. The author always forgets some situations, forcing you to continue to examine simple questions, which can be automatically processed.

Debate with style guide

The debate about style is a waste of time for censorship. Consistent style is really important, but code review is not the time to debate parenthesis. The best way to eliminate style disputes in censorship is to follow a style guide.

A good style guide not only defines surface elements such as naming conventions or blank rules, but also defines how to use the characteristics of a given programming language. For example, JavaScript and Perl contain some functions — they provide many ways to achieve the same logic. Style refers toSouth defines the only way to do things, so that half of the team members do not use a set of linguistic features while the other half uses a completely different set of features.

Once you have a style guide, you don’t need to waste a review cycle to argue with the author about who has the best habit of naming. Just follow the style guide and go on. If your style guide does not specify the conventions of a particular problem, it is generally not worth arguing. If you come across a style guideThe problem involved is also important to discuss and to work with the team. Then add the decision to the style guide so that you never need to have another discussion.

Choose 1: adopt an existing style guide

If you search from the Internet, you can find the published style guide available. Google’s programming style guide is the most famous, but if its style is not suitable for you, you can find other guidebooks. By adopting an existing guide, there is no need to create a large number of styling guidelines from scratch.It can inherit its benefits.

The disadvantage is that organizations optimize their style guides for their own particular needs. For example, the Google style guide is more conservative in the use of new language features, because they have a huge code library, where the code is running on everything, from home routers to the latest iPhone. asIf you are a four person group with only one product, you may choose to be more daring when using frontier language features or extensions.

Choose 2: create your own style guide

If you don’t want to adopt the existing guidelines, you can create one yourself. In the code review, each style is debated and the whole team is asked to decide what the official agreement should be. When you reach a consensus, put the decision into the style guide.

I tend to use the team style guide as a source control Markdown (for example, GitHub page). In this way, any change to the style guide requires a general review process – someone has to make a clear approval of the change, and everyone in the team has the opportunity to make doubts. WBoth ikis and Google files are acceptable options.

Selection 3: mixing method

Merge selection 1 and select 2, you can take the existing style guide as the foundation, then expand or cover the foundation with the local style guide. A good example is Chromium’s C++ style guide. It is guided by the C++ style of GoogleFoundation, but add its own changes and attaching to it.

Start the review right away

Look at code review as a high priority. When you really read the code and feedback, slow down, but start censorship immediately – preferably in a few minutes.

If the team sends you a change list, this may mean that they will be stuck in other jobs until you finish the examination. In theory, the source control system enables the author to build new branches, continue to work, and then incorporate changes into new branches from censorship. In fact, a total of about four developers can do it efficiently.This matter。 Others spend a lot of time cleaning up the three party differences, so that they can offset the progress in waiting for the completion of the review.

You start reviewing immediately, creating a virtuous circle. Your censorship time has completely become a function related to the size and complexity of the change table of the author. This motivates the author to send short and narrow change tables. For you, such a change chart is easier and more enjoyable to review, so you can do it quicker.Review, the cycle continues.

Imagine that your team will perform a new feature, which requires 1000 lines of code change. If they know that you can finish a review of a 200 line change table within about 2 hours, they can split the features into a change table that contains 200 rows, and then in one or two days.Internal inspection is a complete feature. But if you take a day to complete all the code reviews, it takes a week to check the full features. Your players do not want to sit for a week, so they are motivated to send more code reviews, for example, each containing 500 to 600.That’s ok。 Such a review costs much more and gives less feedback, because the background of the 600 line change table is harder than the 200 line change table.

The maximum cycle of a review cycle should be a working day. If you are dealing with a higher priority problem, you can’t complete a review cycle in one day, let your team know and give them the opportunity to give the review to others. If you are forced to refuse censorship more than once a month, it may mean you.The team needs to slow down so that you can keep your mind developing.

From the high level, step down

In an established review cycle, the more annotations you write, the greater the risk that the writer will feel depressed. The exact boundaries vary with the developer, but 20 to 50 annotations in a review cycle are usually the beginning of the danger zone.

If you are worried about drowning the author in the sea of annotations, restrict yourself to feedback in the early cycle of high level problems. Pay attention to redesigning the class interface or splitting complex functions. Until these problems are solved, we can deal with low-level problems, such as variable naming or clarity of code reviews.

Once the author integrates your high-level annotation, low-level comments may become meaningless. Postpone a low – level annotation to a later cycle, and you can save yourself from a carefully worded job, so that the author does not deal with unnecessary notes. This technique also subdivides the attention you are drawn to in the process of review.The image tier helps you and the author complete the change list in a clear and systematic way.

Generously use code examples

In an ideal world, the code author will appreciate every examination received. This is an opportunity for them to learn and prevent them from making mistakes. In fact, there are many external factors that can lead the author to read negative reviews and resent them for annotation. Maybe they are facing the pressure of the deadline, so apart fromEverything that is immediately approved without censorship is a hindrance. Maybe you haven’t worked together, so they don’t believe your feedback is good.

A way to make the writer feel good about the review process is to find opportunities to give them gifts during the examination. What are the gifts that all developers love to receive? It’s the code example, of course.

If you write some suggested changes to lighten the burden of the author, it proves that as a censor, you are generous to time.

For example, imagine a colleague who is not familiar with the list comprehension feature of Python. They sent you a review with the following code:

urls = []

for path in paths:

  url = ‘https://’

  url += domain

  url += path

  urls.append(url)

 

Can you simplify this by using list comprehension? They will be distressed because they have to spend 20 minutes searching for things they never used before.

They will be happier if they receive comments like these:

Consider using list comprehension as a simplification:

urls = [‘https://’ + domain + path for path in paths]

 

This technique is not limited to single command programs. I will often set up my own code branches to show a large proof of the concept to the author, such as splitting a large function or adding a unit test to cover an additional boundary condition.

Preserve this technique for clear and undisputed improvements. In the above list (list comprehension) example, very few developers will refuse to reduce the number of lines of code by 83%. On the contrary, if you write a lengthy example to demonstrate a change “better”, and this change.It’s based on your own personal taste (for example, style change). Code examples make you seem opinionated rather than generous.

Limit yourself to writing only two to three code examples in each review cycle. If you start writing the entire change list for the author, it means you think the author is unable to write his own code.

Never say “you”

It sounds strange, but listen to me: never use the word “you” in code review.

The decision in the review should be based on what makes the code better, not the idea. Your players have poured a lot of effort into their change tables, and are likely to be proud of their work. They heard criticism of their work, and the natural reaction was to pose defensive and protective posture.

Organizational feedback is used to minimize the risk of arousing team members’ alerting. Make it clear that you are criticizing the code, not the programmer. When the author sees the word “you” in the comments, you will shift their attention from code to yourself. This increases their risk of privatization of criticism.

Consider this harmless comment:

You misspelled “successfully”.

The author can interpret this notation as two different meanings:

  • Understand 1:Hey, good guy! You misspelled “successfully”. But I still think you are smart! That might be a slip of the pen.

  • Understand 2:You misspelled “successfully”, stupid.

Compare this with the annotations of “you”.

sucessfully -> successfully

The latter is a simple amendment rather than a trial of the author.

Fortunately, it is not difficult to avoid using “you” when rewriting feedback.

Choose 1: replace “you” with “us”

youCan you rename this variable to make it more descriptive? For example, seconds_remaining.

Become:

WeCan you rename this variable to make it more descriptive? For example, seconds_remaining.

“We strengthened the team’s collective responsibility for code. The author may jump to a different company, and you may, but the team that owns the code will always exist in different forms. When you obviously expect the author to do something on his own, he says, “we” sound silly, but stupid is better than blame..

Select 2: remove the subject of the sentence

Another way to avoid using “you” is to simplify the sentence by omitting the subject of the sentence.

Recommending renamingFor more descriptive names, such as “seconds_remaining”.

You can use the passive voice to achieve similar effects. In technical writing, I usually avoid using the passive voice like plague, but it is a useful way to avoid using “you”.

variableShould be renamedFor more descriptive names, such as “seconds_remaining”.

Another option is to express it as a problem. How “or” “… How “start:”

Rename a variable to a more expressive nameHow?For example, seconds_remaining.

Express the feedback as a request, not an instruction.

Code review requires more wit and caution than ordinary communication, because there is high risk to turn discussions into private arguments. You will expect the censors to show politeness in the examination, but strangely, I find them heading for another direction. Most people would never say to their colleagues, “give me stapler.Give me a bottle of soda. ” But I have seen countless censors using similar instructions to express feedback, for example, “move this class into a separate file.”

I’d rather be annoyed than a gentleman in the feedback. Put an annotation as a request or suggestion rather than an instruction.

Compare the same annotation in two different ways:

Feedback expressed as an instructionFeedback expressed as a request
Move the Foo class into a separate file.Can we move the Foo class into a separate file?

People like to take control of their work. Asking the author for help gives them a sense of autonomy.

The request also makes it easier for the author to respond politely. Maybe their choice is reasonable. If feedback is expressed as an instruction, any feedback from the author is like a violation of instruction. If you express your feedback as a request or question, the author can answer you simply.

The degree of combative dialogue depends on how the examiners express their initial annotations.

Feedback (aggressive) expressed as an instructionFeedback (cooperative) expressed as a request
Examiner:Move the Foo class into a separate file
author:I don’t want to do this because it’s too far away from the Bar class. Customers almost always call them together.
Examiner:Can we move the Foo class into a separate file?
author:Yes, but that’s too far from the Bar class, and the customers usually use these two classes together. What’s your opinion?

Let’s see how polite the dialogue becomes when you build a virtual dialogue to prove that your comments are expressed as requests instead of instructions.

Associate annotations with principles, not opinions.

When you write an annotation to the author, you should give a change proposal as well as a reason for the change. “Now, this class is responsible for downloading files as well as parsing files. We should split it into a download class and an analytic class according to the single responsibility principle. It would be better, not to say, “we should take it.”This class is divided into two. “

Let your remarks have a principled foothold, so that the discussion can move towards a more positive direction and more constructive. But you have a specific reason, such as “we should write this function as a private function to minimize the public excuse class.” the author can’t simply reply, “no, I’m inclined to me.”Method. ” To be more precise, they can, but because you demonstrate how the changes meet the goals, they only show a preference, they will look silly.

Software development is both art and science. You can’t always use definite principles to express clearly what’s wrong with the code. Sometimes code is ugly or unintuitive, and it’s not easy to determine why. Under these circumstances, explain what you can do, but maintain objectivity. If you say “IThis is not easy to understand, which is at least an objective statement; on the contrary, “thisInexplicable is a value judgment that does not necessarily apply to everyone.

Provide support evidence in the form of links as much as possible. The relevant part of the team style guide is the best link you can provide. You can also link to the language or library files. The answer is StackOverflow, but the farther away the authority is, the more unstable your evidence becomes.

The second part: will be on line

Please look forward to other small skills, including:

  • Handling particularly large code reviews

  • Identify the opportunity to give praise

  • Respect for the audit, and

  • Dissolve the deadlock

Edited by Samantha Mason. The illustration comes from Loraine Yow. Thank you @global4g for providing valuable feedback for the early version of this article.

Do you think this article is helpful? Please share it with more people

Focus on “algorithmic enthusiasts” and practice programming internal work

Similar Posts:

Leave a Reply

Your email address will not be published. Required fields are marked *