代码审查需要看懂代码吗_出色的代码审查-团队需要的超级大国

代码审查需要看懂代码吗

There is a general consensus that code reviews are an important aspect of highly effective teams. This research paper is one of many exploring this subject. Most organizations undergo code reviews of some form.

人们普遍认为，代码审查是高效团队的重要方面。 该研究论文是探索该主题的众多论文之一。 大多数组织都会接受某种形式的代码审查。

However, it’s all too common to see code reviews that barely scratch the surface, or that offer feedback that is unclear or hard to act upon. This robs the team the opportunity to speed up learning, share knowledge and context, and raise the quality bar on the resulting code.

但是，常见的情况是看到代码审查几乎没有触及表面，或者提供了不清楚或难以采取行动的反馈。 这使团队失去了加快学习速度，共享知识和上下文以及提高结果代码质量的机会。

At Shopify, we want to move fast while building for the long term. In our experience, having strong code review practices has a huge impact on the growth of our engineers and in the quality of the products we build.

在Shopify，我们希望在长期建设的同时Swift前进。 根据我们的经验，拥有强大的代码审查实践对我们工程师的成长和我们所生产产品的质量有着巨大的影响。

可怕的场景 (A Scary Scenario)

Imagine you join a new team and you’re given a coding task to work on. Since you’re new on the team, you really want to show what you’re made of. You want to perform. So, this is what you do:

想象一下，您加入了一个新团队，并获得了编码任务以进行工作。 由于您是团队中的新成员，因此您真的想展示自己的才能。 你想表演。 因此，这就是您要做的：

1. You work frantically on your task for 3 weeks.
   您疯狂地工作了三个星期。
2. You submit a Pull Request for review with about 1000 new lines of code
   您提交了一个带有大约1000行新代码的Pull Request进行审查
3. You get a couple comments about code style and a question that shows the person has no clue what this work is about.
   您会收到有关代码样式的一些注释，以及一个表明该人员不知道此工作内容的线索的问题。
4. You get approval from both reviewers after fixing the code style and answering the question.
   固定代码样式并回答问题后，您将获得两位审阅者的批准。
5. You merge your branch into master, eyes closed, shoulders tense, grinding your teeth. After a few minutes, CI completes. Master is not broken. Yet.
   您将分支合并为主分支，闭上眼睛，肩膀紧绷，磨牙。 几分钟后，配置项完成。 师父没有坏。 然而。
6. You live in fear for 6 months, not knowing when and how your code will break.
   您在恐惧中生活了6个月，不知道您的代码何时以及如何中断。

You may have lived through some of the situations above, and hopefully you’ve seen some of the red flags in that process.

您可能已经经历了上述某些情况，并且希望您已经看到了该过程中的一些危险信号。

Let’s talk about how we can make it much better.

让我们谈谈如何使它变得更好。

规范审查实践 (Practical Code Review Practices)

At Shopify, we value the speed of shipping, learning, and building for the long term. These values — which sometimes conflict — lead us to experiment with many techniques and team dynamics. In this article, I have distilled a series of very practical techniques we use at Shopify to ship valuable code that can stand the test of time.

在Shopify，我们重视长期的运输，学习和建造速度。 这些有时会冲突的价值观导致我们尝试许多技术和团队动力。 在本文中，我总结了Shopify所使用的一系列非常实用的技术，以交付可以经受时间考验的有价值的代码。

A Note about terminology: We refer to Pull Requests (PR) as one unit of work that’s put forth for review before merging into the base branch. GitHub and Bitbucket users will be familiar with this term.

有关术语的注释：我们将“拉取请求(PR)”作为合并到基础分支之前进行审核的一个工作单元。 GitHub和Bitbucket用户将熟悉此术语。

1.保持较小的拉取请求 (1. Keep Your Pull Requests Small)

As simple as this sounds, this is easily the most impactful technique you can follow to level up your code review workflow. There are 2 fundamental reasons why this works:

听起来很简单，但这是您可以用来升级代码审查工作流的最有影响力的技术。 起作用的原因有两个：

• It’s mentally easier to start and complete a review for a small piece. Larger PRs will naturally make reviewers delay and procrastinate examining the work, and they are more likely to be interrupted mid-review.

  从精神上讲， 开始并完成一小部分的评论比较容易。 较大的PR会自然地使审稿人延迟并拖延检查工作，并且他们更有可能在审阅中被打断。

• As a reviewer, it’s exponentially harder to dive deep if the PR is long. The more code there is to examine, the bigger the mental map we need to build to understand the whole piece.

  作为审稿人，如果公关时间长， 深入研究难度就成倍增加。 要检查的代码越多，我们需要构建的心理图就越大，以理解整个过程。

Breaking up your work in smaller chunks increases your chances of getting faster and deeper reviews.

将您的工作分解成小块，可以增加获得更快，更深入的评论的机会。

Now, it’s impossible to set one universal standard that applies to all programming languages and all types of work. Internally, for our data engineering work, the guideline is around 200–300 lines of code affected. If we go above this threshold, we almost always break up the work into smaller blocks.

现在，不可能设置适用于所有编程语言和所有类型工作的通用标准。 在内部，对于我们的数据工程工作，该准则涉及大约200-300行代码。 如果我们超过此阈值，我们几乎总是将工作分解成较小的块。

Of course, we need to be careful about breaking up PRs into chunks that are too small, since this means reviewers may need to inspect several PRs to understand the overall picture.

当然，我们需要谨慎地将PR分成太小的块，因为这意味着审阅者可能需要检查几个PR才能了解整体情况。

2.使用草稿PR (2. Use Draft PRs)

Have you heard the metaphor of building a car vs. drawing a car? It goes something like this:

您是否听说过建造汽车与绘制汽车的比喻？ 它是这样的：

1. You’re asked to build a car.
   您被要求制造汽车。
2. You go away for 6 months and build a beautiful Porsche.
   您要离开6个月，然后再造一辆漂亮的保时捷。
3. When you show it to your users, they ask about space for their 5 children and the surf boards.
   当您向用户展示时，他们询问有关5个孩子和冲浪板的空间。

Clearly, the problem here is that the goal is poorly defined and the team jumped directly into the solution before gathering enough feedback. If, after step one, we created a drawing of the car and showed it to our users, they would have asked the same questions and we would have discovered their expectations and saved ourselves 6 months of work. Software is no different; we can make the same mistake and work for a long time on a feature or module that isn’t what our users need.

显然，这里的问题是目标定义不明确，团队在收集足够的反馈之前直接跳入了解决方案。 如果在第一步之后，我们创建了汽车的图纸并将其展示给我们的用户，那么他们会问同样的问题，我们就会发现他们的期望并节省了6个月的工作时间。 软件没有什么不同； 我们可能会犯同样的错误，并长时间使用用户不需要的功能或模块。

At Shopify, it’s common practice to use Work In Progress (WIP) PRs to elicit early feedback whose goal is validating direction (choice of algorithm, design, API, etc). Early changes mean less wasted effort on details, polish, documentation, etc.

在Shopify，通常的做法是使用进行中的工作(WIP)PR来引起早期反馈，其目标是验证方向(选择算法，设计，API等)。 早期更改意味着减少了对细节，修饰，文档等方面的浪费。

As an author, this means you need to be open to changing the direction of your work. At Shopify, we try to embrace the principle of strong opinions, loosely held. We want people to make decisions confidently, but also be open to learning new and better alternatives, given sufficient evidence. In practice, we use Github’s Draft PRs — they clearly signal the work is still in flow and Github prevents you from merging a Draft PR. Other tools may have similar functionality, but at the very least you can create normal PRs with a clear WIP label to indicate the work is early stage. This will help your reviewers focus on offering the right type of feedback.

作为作者，这意味着您需要开放以改变工作方向。 在Shopify，我们尝试采用松散持有强烈意见的原则。 我们希望人们自信地做出决定，但也要在有足够证据的情况下愿意学习新的更好的选择。 实际上，我们使用Github的PR草案 -它们清楚地表明工作仍在进行中，并且Github阻止您合并PR草案。 其他工具可能具有类似的功能，但是至少您可以创建带有清晰WIP标签的正常PR，以指示工作尚处于初期阶段。 这将帮助您的审稿人专注于提供正确类型的反馈。

3.每个关注一个公关 (3. One PR Per Concern)

In addition to line count, another dimension to consider is how many concerns your unit of work is trying to address. A concern may be a feature, a bugfix, a dependency upgrade, an API change, etc. Are you introducing a new feature while refactoring at the same time? Fixing two bugs in one shot? Introducing a library upgrade and a new service?

除了行数之外，还要考虑的另一个维度是您的工作单元要解决的问题有多少。 一个问题可能是功能，错误修正，依赖性升级，API更改等。您是否在同时重构的同时引入了新功能？ 一枪解决两个错误？ 引入了库升级和新服务？

Breaking down PRs into individual concerns has the following effects:

将PR分解为个人问题具有以下效果：

• More independent review units and therefore better review quality

  独立的审查单位更多 ，因此审查质量更高

• Fewer affected people, therefore less domains of expertise to gather

  受影响的人较少 ，因此可以收集的专业知识较少

• Atomicity of rollbacks, the ability of rolling back a small commit or PR. This is valuable because if something goes wrong, it will be easier to identify where errors were introduced and what to roll back.

  回滚的原子性，回滚小的提交或PR的能力。 这很有价值，因为如果出现问题，将更容易识别错误的出处和回退的内容。

• Separating easy stuff from hard stuff. Imagine a new feature that requires refactoring a frequently used API. You change the API, update a dozen call-sites, and then implement your feature. 80% of your changes are obvious and skimmable with no functional changes, while 20% are new code that needs careful attention to test coverage, intended behaviour, error handling, etc. and will likely go through multiple revisions. With each revision, the reviewer will need to skim through all of the changes to find the relevant bits. By splitting this in two PRs, it becomes easy to quickly land the majority of the work and to optimize the review effort applied to the harder work.

  将容易的东西与困难的东西分开 。 想象一下一项新功能，该功能需要重构常用的API。 您更改API，更新十二个呼叫站点，然后实施您的功能。 您有80％的更改是显而易见的，可略读而没有任何功能更改，而20％是新代码，需要仔细关注测试范围，预期行为，错误处理等，并且可能会进行多次修订。 对于每个修订版，审阅者将需要浏览所有更改以找到相关的位。 通过将其分为两个PR，可以轻松快速地完成大部分工作，并优化应用于较艰苦工作的审核工作。

If you end up with a PR that includes more than one concern, you can break it down into individual chunks. Doing so will accelerate the iteration cycle on each individual review, giving a faster review overall. Often part of the work can land quickly, avoiding code rot and merge conflicts.

如果最终的PR涉及多个问题，则可以将其细分为各个部分。 这样做将加快每个单独审阅的迭代周期，从而使总体审阅速度更快。 通常，部分工作可以Swift完成，避免代码腐烂和合并冲突。

In the example above, we’ve taken a PR that covered three different concerns and broke it up. You can see how each reviewer has strictly less context to go over. Best of all, as soon as any of the reviews is complete, the author can begin addressing feedback while continuing to wait for the rest of the work. In the most extreme cases, instead of completing a first draft, waiting several days (and shifting focus), and then eventually returning to address feedback, the author can work almost continuously on their family of PRs as they receive the different reviews asynchronously.

在上面的示例中，我们采用了涵盖三个不同问题的PR，并将其分解。 您可以看到每个审阅者如何严格限制上下文。 最重要的是，一旦任何评论完成，作者就可以开始处理反馈，同时继续等待其余的工作。 在最极端的情况下，当作者异步地收到不同的评论时，他们可以几乎连续地处理其PR系列，而不必完成初稿，等待几天(并转移焦点)，然后最终返回到反馈意见。

4.专注于守则，而不是人 (4. Focus on the Code, Not the Person)

Focus on the code, not the person practice refers to communication styles and relationships between people. Fundamentally, it’s about trying to focus on making the product better, and avoiding the author perceiving a review as personal criticism.

专注于代码，而不是人的实践是指人与人之间的沟通方式和关系。 从根本上讲，它是关于着重于使产品更好，并避免作者将评论视为个人批评。

Here are some tips you can follow:

您可以按照以下提示操作：

• As a reviewer, think, “This is our code, how can we improve on it?”

  作为审阅者，请考虑：“这是我们的代码，我们如何对其进行改进？”

• Offer positive remarks! If you see something done well, comment on it. This reinforces good work and helps the author balance suggestions for improvement.
  提供正面评价！ 如果您看到做得很好，请对此发表评论。 这加强了良好的工作，并帮助作者平衡了改进建议。
• As an author, assume best intention, and don’t take comments personally.
  作为作者，请秉承最好的意图，不要亲自发表评论。

Below are a few examples of not-so-great review comments, and a suggestion on how we can reword to emphasize the tips above.

以下是一些不太好的评论评论示例，以及关于如何改写以强调上述提示的建议。

Ultimately, a code review is a learning and teaching opportunity and should be celebrated as such.

归根结底，代码审查是一个学习和教学的机会，应这样庆祝。

5.选择合适的人进行审核 (5. Pick the Right People to Review)

It’s often challenging to decide who should review your work. Here are some questions can use as guidance:

决定谁应该审查您的工作通常很困难。 以下是一些可以用作指导的问题：

• Who has context on the feature or component you’re building?
  谁对您正在构建的功能部件或组件具有上下文？
• Who has strong skills in the language, framework, or tool you’re using?
  谁在您使用的语言，框架或工具方面拥有强大的技能？
• Who has strong opinions on the subject?
  谁对这个问题有很强的见解？
• Who cares about the result of what you’re doing?
  谁在乎你在做什么的结果？
• Who should learn this stuff? Or if you’re a junior reviewing someone more senior, use this as an opportunity to ask questions and learn. Ask all the silly questions, a strong team will find the time to share knowledge.
  谁应该学习这些东西？ 或者，如果您是初中级的人，可以借此机会提问和学习。 提出所有愚蠢的问题，强大的团队就会找到时间分享知识。

Whatever rules your team might have, remember that it is your responsibility as an author to seek and receive a high-quality code review from a person or people with the right context.

无论您的团队可能有什么规则，请记住，作为作者的责任是从一个或多个具有适当背景的人员那里寻求并接受高质量的代码审查。

6.给您的评论者一张地图 (6. Give Your Reviewers a Map)

Last but definitely not least, the description on your PR is crucial. Depending on who you picked for review, different people will have different context. The onus is on the author to help reviewers by providing key information or links to more context so they can produce meaningful feedback.

最后但同样重要的是，您的PR上的描述至关重要。 根据您选择的审核对象，不同的人会有不同的背景。 作者有责任通过提供关键信息或更多上下文的链接来帮助审稿人，以便他们产生有意义的反馈。

Some questions you can include in your PR templates:

您可以在PR模板中包含一些问题：

• Why is this PR necessary?
  为什么需要此PR？
• Who benefits from this?
  谁从中受益？
• What could go wrong?
  可能出什么问题了？
• What other approaches did you consider? Why did you decide on this approach?
  您还考虑其他哪些方法？ 您为什么决定采用这种方法？
• What other systems does this affect?
  这还会影响其他哪些系统？

Good code is not only bug-free; it is also useful! As an author, ensure that your PR description ties your code back to your team’s objectives, ideally with link to a feature or bug description in your backlog. As a reviewer, start with the PR description; if it’s incomplete, send it back before attempting to judge the suitability of the code against undefined objectives. And remember, sometimes the best outcome of a code review is to realize that the code isn’t needed at all!

好的代码不仅没有错误； 这也很有用！ 作为作者，请确保您的PR描述将您的代码与团队的目标联系起来，最好在待办事项中链接到功能或错误描述。 作为审稿人，请从PR说明开始； 如果代码不完整，请先将其发送回去，然后再根据未定义的目标来判断代码的适用性。 请记住，有时代码审查的最好结果是意识到根本不需要代码！

有什么好处？ (What’s the Benefit?)

By adopting some of the techniques above, you can have a strong impact on the speed and quality of your software building process. But beyond that, there’s the potential for a cultural effect:

通过采用上述某些技术，可以对软件构建过程的速度和质量产生重大影响。 但除此之外，还可能产生文化影响：

• Teams will build a common understanding. The group understands your work better and you’re not the only person capable of evolving any one area of the codebase.

  团队将建立共识 。 该小组更好地了解您的工作，并且您不是唯一能够发展代码库任何一个领域的人。

• Teams will adopt a sense of shared responsibility. If something breaks, it’s not one person’s code that needs fixing. It’s the team’s work that needs fixing.

  团队将采取分担责任感 。 如果出现问题，则不需要修正某个人的代码。 团队的工作需要解决。

Any one person in a team should be able to take a holiday and disconnect from work for a number of days without risking the business or stressing about checking email to make sure the world didn’t end.

团队中的任何一个人都应该能够休假并中断几天的工作，而不必冒险做生意或强调检查电子邮件以确保世界不会终结。

我该怎么做才能改善团队的代码审查流程？ (What Can I Do to Improve My Team’s Code Review Process?)

If you lead teams, start experimenting with these techniques and find what works for your team.

如果您领导团队 ，请开始尝试这些技术，然后找到适合您团队的方法。

If you’re an individual contributor, discuss with your lead on why you think code reviews techniques are important, how they help effectiveness and how they help your team.

如果您是个人贡献者 ，请与您的主管讨论为什么您认为代码审查技术很重要，它们如何帮助有效性以及如何帮助您的团队。

Bring this up on your next 1:1 or your next team synch.

在下一个1：1或下一个团队同步时进行。

代码审查的重要性 (The Importance of Code Reviews)

To close, I’ll share some words from my lead, which summarizes the importance of Code Reviews:

最后，我将分享一些潜在客户的话，总结了代码审查的重要性：

“We could prioritize landing mediocre but working code in the short term, and we will write the same debt-ridden code forever, or we can prioritize making you a stronger contributor, and all of your future contributions will be better (and your career brighter).

“我们可以在短期内优先考虑平庸但工作正常的代码，我们将永远编写相同的债务缠身代码，或者我们可以优先考虑使您成为更强大的贡献者，并且您将来的所有贡献都会更好(并且您的事业更加光明)。

An enlightened author should be delighted to have this attention.”

开明的作者应该很高兴获得如此关注。”

We’re always on the lookout for talent and we’d love to hear from you. Please take a look at our open positions on the Data Science & Engineering career page.

我们一直在寻找人才，我们很高兴收到您的来信。 请在数据科学与工程职业页面上查看我们的空缺职位。

Originally published at https://engineering.shopify.com.

最初发布在 https://engineering.shopify.com上 。

翻译自: https://medium.com/data-shopify/great-code-reviews-the-superpower-your-team-needs-933bcb84f5e

代码审查需要看懂代码吗