winterggg / blog

0 stars 0 forks source link

代码评审 - the missing readme #3

Open winterggg opened 5 months ago

winterggg commented 5 months ago

代码评审

本文取材自《The missing readme》-- Chapter 6

当你是被评审者

精心准备

你的目的是让评审者理解你在做什么以及提供有建设性的反馈,有些建议可以遵循:

  1. 提交 pr 前确保通过了 test 和 lint;
  2. 单次 pr 的改动不要太大;
  3. 将 feature 和 refactor 分到不同的 pr 里;
  4. 需要给出描述性的提交信息;
  5. 务必将注释和测试包括在内;
  6. 不要守护你提所交的代码,要期待它在评审中发生变化,有时候甚至是巨大的变化。

walking-through 会议

对于大体量的代码修改,需要安排 walking-through 会议,内容主要是快速回顾设计文档,在 IDE 里以某种顺序展示代码。解释任何新模型或抽象背后的主要概念、它们是如何被使用的,以及它们是如何整合的。这个会议的目的是让团队快速理解为何要提出修改,并给他们一个良好的心理模型。

(这个对于简道云有点高级了,不过我感觉 walking-through 会议的一些信息完全可以在 PR 描述里传递)

保持主动

一方面不要羞于要求别人评审你的代码,另一方面收到评论后要及时回应,你回应得越快,得到他人回应的速度就越快。

不要太在意

给自己洗脑 -- 这些评审意见是针对代码的,而不是针对你个人的。

得到很多建议不意味着你没有通过考验,只意味着评审者正在参与到你的代码中并且在思考如何去改进它。

尽量保持开放的心态。

关于如何看待评审,摘抄一段不久前刷到的文章:

代码评审是我们工程师通行的质量管理手段。它基于这么一个事实:不管一个人有多聪明多细心,他总是考虑得不完善,需要同事提供 peer review。因此它也暗含了这么一个规则:一个人的代码被驳回或者被要求重写,并不代表此人水平不行。代码评审过程中的意见不应该被视作对原作者的批评。这条规则被广泛采纳,大名鼎鼎的 Linus 就因为喜欢在代码评审的时候攻击原作者,被社区停职了一段时间。

当你是评审者

分流和预留时间

按照重要程度来排序,同时对于一些小改动也可以快速评审。

提前在日历上规划评审时间。如果收到一个评审需要花费一两个小时,创建一个 JIRA 任务跟踪代码评审本身。

提供全面的反馈

包括:正确性、可实施性、可维护性、可读性和安全性。要承认优点。

区分问题、建议和挑剔:简单来说,将提出的建议与真正希望的修改区分开来。比如可以在一些建议前加上「可选」、「Nit」之类的。

不要忘记评审测试代码:从测试代码开始评审是比较好的实践,它们说明了代码是如何被使用的,以及预期会发生什么。

尊重当前的修改范围,不要给出一些明显超出范围的修改意见,如有需要可以创建任务。

在 IDE 里评审

一方面大型的改动在网页端很难浏览;另一方面本地代码可以运行,可以创建测试用例来验证,甚至构造失败的场景,以更好的说明评审意见。