Skip to content
体验新版
项目
组织
正在加载...
登录
切换导航
打开侧边栏
xindoo
eng-practices-cn
提交
a91b0915
E
eng-practices-cn
项目概览
xindoo
/
eng-practices-cn
通知
1
Star
0
Fork
0
代码
文件
提交
分支
Tags
贡献者
分支图
Diff
Issue
0
列表
看板
标记
里程碑
合并请求
0
Wiki
0
Wiki
分析
仓库
DevOps
项目成员
Pages
E
eng-practices-cn
项目概览
项目概览
详情
发布
仓库
仓库
文件
提交
分支
标签
贡献者
分支图
比较
Issue
0
Issue
0
列表
看板
标记
里程碑
合并请求
0
合并请求
0
Pages
分析
分析
仓库分析
DevOps
Wiki
0
Wiki
成员
成员
收起侧边栏
关闭侧边栏
动态
分支图
创建新Issue
提交
Issue看板
前往新版Gitcode,体验更适合开发者的 AI 搜索 >>
提交
a91b0915
编写于
10月 26, 2019
作者:
xindoo
浏览文件
操作
浏览文件
下载
电子邮件补丁
差异文件
update translations
上级
279c7482
变更
12
隐藏空白更改
内联
并排
Showing
12 changed file
with
78 addition
and
92 deletion
+78
-92
review/developer/cl-descriptions.md
review/developer/cl-descriptions.md
+14
-19
review/developer/handling-comments.md
review/developer/handling-comments.md
+1
-1
review/developer/index.md
review/developer/index.md
+2
-2
review/developer/small-cls.md
review/developer/small-cls.md
+8
-13
review/index.md
review/index.md
+13
-13
review/reviewer/comments.md
review/reviewer/comments.md
+8
-10
review/reviewer/index.md
review/reviewer/index.md
+5
-5
review/reviewer/looking-for.md
review/reviewer/looking-for.md
+3
-4
review/reviewer/navigate.md
review/reviewer/navigate.md
+9
-10
review/reviewer/pushback.md
review/reviewer/pushback.md
+3
-3
review/reviewer/speed.md
review/reviewer/speed.md
+9
-9
review/reviewer/standard.md
review/reviewer/standard.md
+3
-3
未找到文件。
review/developer/cl-descriptions.md
浏览文件 @
a91b0915
# 写一个好的CL描述
一个CL描述是做了
**什么**
更改以及
**为什么**
的公开记录。
它将成为我们版本控制历史的永久组成部分,多年来除你的
reviewer
以外,还可能有数百人会阅读它。
它将成为我们版本控制历史的永久组成部分,多年来除你的
评审者
以外,还可能有数百人会阅读它。
将来的开发者将会基于它的描述来搜索你的CL。
将来可能会有人由于没有现成的细节,而根据他模糊的记忆来寻找你的改变。
将来可能会有人由于没有现成的细节,而根据他模糊的记忆来寻找你的改变。
如果所有重要的细节都在代码中而不是描述中,那么他们将很难定位你的CL。
## 第一行
{#firstline}
## 第一行
*
简短的摘要:将要做些什么
*
完整的句子:将它写成一个命令(祈使句)
*
接着紧跟一个空行
*
言简意赅
*
语义完整
*
空行隔开
CL描述的
**第一行**
应该是对CL正在
**做的**
*具体*
工作的简短总结,紧跟一个空行。
在未来,这是大多数的代码搜索者在浏览一段代码的版本控制历史时会看到的,因此第一行应该足够有信息量,他们不必阅读你的CL或它的整个描述就可以大致了解你的CL实际做了什么。
...
...
@@ -18,17 +18,13 @@ CL描述的**第一行**应该是对CL正在**做的***具体*工作的简短总
例如,说
\"
**Delete**
the FizzBuzz RPC and
**replace**
it with the new system.",而不是
\"
**Deleting**
the FizzBuzz RPC and
**replacing**
it with the new system."。
不过,你不必把其余的描述写成祈使句。
## 正文内容丰富
{#informative}
## 正文内容丰富
其余描述应该是具有丰富的内容。它可能包括对正在解决的问题的简要描述,以及为什么这是最好的方法。
如果这种方法有任何缺点,就应该提到。
将相关的背景信息(例如bug数目,基准测试结果),以及设计文档的链接。
即使是很小的CL也值得关注细节。请将来龙去脉放入CL中。
## 不好的CL描述 {#bad}
其余描述应该是具有丰富的内容。它可能包括对正在解决的问题的简要描述,以及为什么这是最好的方法。 如果这种方法有任何缺点,就应该提到。将相关的背景信息(例如bug数目,基准测试结果),以及设计文档的链接。即使是很小的CL也值得关注细节。请将来龙去脉放入CL中。
## 反例
"Fix bug"是一个不充分的CL描述。是什么bug?你是怎么修复它的?
其他一些
相
似的不好的CL描述:
其他一些
类
似的不好的CL描述:
-
"Fix build."
-
"Add patch."
...
...
@@ -39,7 +35,7 @@ CL描述的**第一行**应该是对CL正在**做的***具体*工作的简短总
这里有一些是真实的CL描述。他们的作者可能认为他们提供的信息是有用的,但他们并没有给出一个CL描述的意图。
## 好的CL描述
{#good}
## 好的CL描述
这里有一些好的CL描述的例子.
...
...
@@ -73,7 +69,7 @@ CL描述的**第一行**应该是对CL正在**做的***具体*工作的简短总
其余的描述将讨论具体的实现、CL的来龙去脉、解决方案的不理想以及未来可能的方向。
它同样是解释
*为什么*
要进行这个修改。
### 需要
背景
的小CL
### 需要
上下文
的小CL
> Create a Python3 build rule for status.py.
>
...
...
@@ -88,7 +84,6 @@ CL描述的**第一行**应该是对CL正在**做的***具体*工作的简短总
## 在提交CL之前,请review描述
在Review期间CL可能会经历重大改变。
在提交CL之前,有必要review CL描述,以确保描述仍然反映CL所做的事情。
在Review期间CL可能会经历重大改变。在提交CL之前,有必要review CL描述,以确保描述仍然反映CL所做的事情。
下一章:
[
小
的CL
](
small-cls.md
)
下一章:
[
简短
的CL
](
small-cls.md
)
review/developer/handling-comments.md
浏览文件 @
a91b0915
...
...
@@ -18,7 +18,7 @@ Code Review的目标是维护代码库和产品的质量。如果评审者批评
如果评审者无法理解你的某部分代码,那边可能未来的阅读者也可能理解不了。在Code Review工具中回应帮不了未来的读者,但是代码中的注释可以。
##
想想你自己
##
自我反思
写一个变更会花费你很大的精力,提价Code Review时会感觉如释负重,而且自己也相当确定所有工作已经做完了。所以当评审者提出改进建议时,你很容易认为那些都是错的,或者认为是评审者给你不必要的阻挠,再或者觉得评审者应该让你提价变更。 无论如何,
**不管你怎么想**
,花点时间回想下评审者给你的反馈有助于提升公司的代码质量。你始终问下自己“如果评审者是对的呢?”
...
...
review/developer/index.md
浏览文件 @
a91b0915
# CL开发者指南 —— 更好的通过
code r
eview
# CL开发者指南 —— 更好的通过
Code R
eview
这一节介绍开发者CL的相关实践,帮助你更快地通过Code Review,并获得更高质量的结果。你不必全部读完,但是它们对每个谷歌开发人员都非常有用,当你全部阅读完成后会觉得非常有用
-
[
写好CL描述
](
cl-descriptions.md
)
-
[
简短的CL
](
small-cls.md
)
-
[
怎么处理审查这的评论建议
](
handling-comments.md
)
-
[
如何处理评审者的建议?
](
handling-comments.md
)
也请阅读
[
怎样去做Code Review
](
../reviewer/
)
, 了解更多有关code reviewer相关的细节。
...
...
review/developer/small-cls.md
浏览文件 @
a91b0915
#
精简
的CL
#
简单
的CL
## 为什么要写成一系列简
练
的CL?
## 为什么要写成一系列简
短
的CL?
精简
的CL有这些好处:
简短
的CL有这些好处:
-
**
code r
eview更快**
与比起花30分钟审查一个大型的CL相比,对审查者来说花5分钟审查一系列小的CL更加容易.
-
**
Code R
eview更快**
与比起花30分钟审查一个大型的CL相比,对审查者来说花5分钟审查一系列小的CL更加容易.
-
**审查更加彻底。**
进行较大的更改后,审阅者和作者往往会因大量详细评论的来回移动而感到沮丧,有时这些评论会漏掉或遗漏重要的观点。
-
**减少导致bug的可能性。**
由于您所做的更改较少,因此您和您的审阅者更容易有效地推断出CL的影响,并查看是否导致bug。
-
**减少不必要的工作**
当你写了一个巨大的CL,然后审查者觉得你总体方向错了,这会导致你浪费大量的工作。
-
**更方便合并代码**
因为大型的CL会导致大量的冲突,因此合并大型的CL会浪费很多时间,并且这将会是你经常做的工作。
-
**有助于你作出更好的设计**
优雅的设计并且完善一个小的改动比大的改动更加容易
-
**
减少审查者的阻碍
**
提审部分改动,不会影响你继续编码。
-
**
降低审查者的难度
**
提审部分改动,不会影响你继续编码。
-
**回滚更容易**
大型CL很有可能会在初始CL提交和回滚CL之间更新修改文件,从而使回滚变得复杂(中型CL也可能也需要回滚可能也会这样)。
注意!
**审查者可以因为你的改动过于巨大直接拒绝掉你**
通常,他们会感谢您的贡献,但要求您以某种方式使它成为一系列较小的更改。不管是你把这些改动拆分成小的改动,还是和审查者争论让他接受都会耗费你大量的时间。但是编写小型改动就不会有这样的问题。
##
What is Small
?
##
怎么样算简短
?
通常,CL的正确大小是
**一个独立的更改**
。 这意味着:
...
...
@@ -29,6 +29,7 @@
多大算大,没有一个明确的要求。 对于CL而言,100行通常是一个合理的大小,而1000行通常太大,但这取决于您的审阅者的判断。 更改分布的文件数量也会影响其“大小”。 可以在一个文件中进行200行的更改,但是将其分布在50个文件中通常会太大。
请记住,尽管从您开始编写代码起就一直与您紧密联系,但审阅者通常没有上下文。 对您来说,看起来像可接受大小的CL可能会让您的审阅者不知所措。毫无疑问,尽可能把CL些小是正确的。审查者很少抱怨CL太小
## 什么时候大型的CL也是可以的?
在某些情况下,较大的更改没有那么糟糕:
...
...
@@ -44,7 +45,7 @@
另一个示例:您发送一个CL进行代码更改,而另一个CL则发送使用该代码的配置或实验; 如果需要,这也更容易回滚,因为有时将配置/实验文件推送到生产环境中比更改代码更快。
##
单独重构
##
把代码重构分离出来
通常重构最好不要和功能修改或者bug fix一起提CL。比如移动或者重命名一个Class,最好和这个Class的bug fix分开提CL。这样对于审阅者来说,理解每个CL单独引入的更改要容易得多。
...
...
@@ -70,12 +71,6 @@
在写大型CL之前,思考下是不是能够将重构分离出来,这是一个更加清晰的思路。多和你的队友交流学习下他们对缩小CL的实践和方法。
If all of these options fail (which should be extremely rare) then get consent
from your reviewers in advance to review a large CL, so they are warned about
what is coming. In this situation, expect to be going through the review process
for a long time, be vigilant about not introducing bugs, and be extra diligent
about writing tests.
如果所有这些选项都失败了(这种情况很少见),那么请事先获得您的审阅者的同意,告诉他们一个巨大的CL将要来临。 在这种情况下,审查过程会耗费极其长的实践,这样请花费更多的时间去写测试代码,避免引入bug。
下一节:
[
怎么处理审阅者的评论
](
handling-comments.md
)
review/index.md
浏览文件 @
a91b0915
#
代码评审
开发者指南
#
Code Review
开发者指南
## 介绍
{#intro}
代码评审
是一些人提交一些代码片段,供另外一些人审阅的流程。
在谷歌,我们都是通过
代码评审
来保证代码和产品的质量。
这篇文档是对谷歌
代码评审
流程和策略的权威描述。
这一页是我们
代码评审
过程的概述。这篇指南其他有两个大的部分,分别是:
## 介绍
Code Review
是一些人提交一些代码片段,供另外一些人审阅的流程。
在谷歌,我们都是通过
Code Review
来保证代码和产品的质量。
这篇文档是对谷歌
Code Review
流程和策略的权威描述。
这一页是我们
Code Review
过程的概述。这篇指南其他有两个大的部分,分别是:
-
**[评审者指南](reviewer/)**
: 代码评审者的详细指南。
-
**[开发者指南](developer/)**
: 提交
变更评审
的开发者的详细指南。
-
**[开发者指南](developer/)**
: 提交
Code Review
的开发者的详细指南。
## 代码评审者需要关注什么?
{#look_for}
## 代码评审者需要关注什么?
代码评审者需要关注:
-
**设计**
: 代码是否设计良好并且适合你们的系统?
...
...
@@ -21,17 +21,17 @@
-
**代码风格**
: 代码风格是否遵循
[
谷歌代码风格指南
](
http://google.github.io/styleguide/
)
?
-
**文档**
: 开发者是否更新了相关文档?
参考
**[
如何做代码评审
](reviewer/)**
获取更多信息。
参考
**[
代码评审者指南
](reviewer/)**
获取更多信息。
### 挑选最适合的代码评审者
{#best_reviewers}
### 挑选最适合的代码评审者
通常而言,你都希望找个一个
*最合适*
的评审者在合理的时间里对你的变更作出评审。
最合适的评审者是那些能对你代码做出最全面最准确评价的人,一般情况下都是代码的维护者(他可能在所有者列表里也有可能不在)。 有时这意味着要不同的人阅读不同的部分。
如果你找到理想的评审者但他没有时间,你也至少应该抄送他。
###
当面评审 {#in_person}
###
亲自评审
如果你是和某个合格的代码评审者结对写的代码,那么这段代码可以认为已经通过评审了。
你也可以
进行面对面的代码评审,评审者提出问题,而变更的开发人员回答问题。
你也可以
以问答的方式亲自参与代码评审。
## 参见
{#seealso}
## 参见
-
**[评审者指南](reviewer/)**
: 代码评审者的详细指南。
-
**[开发者指南](developer/)**
: 提交变更评审的开发者的详细指南。
\ No newline at end of file
review/reviewer/comments.md
浏览文件 @
a91b0915
# 怎样写代码评注
# 怎样写代码评论
## 概要
-
礼貌
.
-
礼貌
-
解释你的观点.
-
明确指出方向和问题,帮助开发人员去权衡作出决定.
-
鼓励开发人员通过注释和精简代码来解决你的困惑而不是通过解释
## 礼貌
## 礼貌
通常来说当你code review代码时保持礼貌和尊重能使开发人员更加清晰,得到更多帮助。这样是为了保证你的代码评论仅仅针对的是
*code*
而不是针对
*开发人员*
。你不必一直这么去做,但是当你的评论会让开发人员生气或者产生争执时有必要这么去做。比如:
...
...
@@ -17,11 +15,11 @@
好的例子: "我并没有发现这个并发模块给程序带来了多少帮助,并且还增加了程序的复杂性,因此我认为这段代码最好是用单线程而不是多线程。
## 解释清楚原因
{#原因}
## 解释清楚原因
从上面“好”的例子当中你能发现,这样有助于开发人员理解
*为什么*
你写了这些评注。你不一定非得包含这些信息在你的评注里面,但是适当的多解释你的意图或者多给出一些提升代码质量的建议都是非常好的实践。
## 给予
知道 {#指导}
## 给予
指导
**通常来说修复CL是开发人员的职责而不是评审人员的**
。你不需要向开发人员提供详细的解决方案或者代码。
...
...
@@ -29,10 +27,10 @@
尽管这样,有时候直接给出指导,建议甚至是代码更有帮助。code review的主要目的是尽可能得到最好的CL。其次是提高开发人员的技能这样就能减少以后评审的次数。
## 接受
注释 {#注释}
## 接受
解释
当你要求开发人员解释一段你看不懂的代码的时候,通常的结果是
**他们写代码,让代码更清晰**
。当一段代码不是太过于复杂的时候通过加一些注释偶尔也是一种不错的做法。
当你要求开发人员解释一段你看不懂的代码的时候,通常的结果是
**他们
重
写代码,让代码更清晰**
。当一段代码不是太过于复杂的时候通过加一些注释偶尔也是一种不错的做法。
**
注释仅仅写在code review工具里面
不利于将来的代码阅读者。**
只有极少数情况是可行的,比如你对你评审的需求不太熟悉,但是开发人员解释的是大多数人都知道的。
**
解释仅仅是写在Code Review工具里的,
不利于将来的代码阅读者。**
只有极少数情况是可行的,比如你对你评审的需求不太熟悉,但是开发人员解释的是大多数人都知道的。
下一节:
[
处理代码评审回推
](
pushback.md
)
review/reviewer/index.md
浏览文件 @
a91b0915
# 如何做
code r
eview?
# 如何做
Code R
eview?
本节中介绍了基于长期经验总结的Code Review最佳
时间方式。所有内容都在同一个章节里,但被分为了好多子章节。你不必全部读一遍,但很多人发现阅读这些内容对自己的团队很有帮助
。
本节中介绍了基于长期经验总结的Code Review最佳
实践。所有内容都在同一个章节里,但被分为了好多子章节。虽然你不必通读一遍,但认真看一遍对你自己和整个团队都会有很大的益处
。
-
[
Code Review的标准
](
standard.md
)
-
[
Code Review应该关注
一些啥
?
](
looking-for.md
)
-
[
Code Review应该关注
什么
?
](
looking-for.md
)
-
[
Code Review的步骤
](
navigate.md
)
-
[
Code Review
s
的速度
](
speed.md
)
-
[
Code Review的速度
](
speed.md
)
-
[
在Code Review中如何写评论
](
comments.md
)
-
[
处理Code Review
s中推延
](
pushback.md
)
-
[
处理Code Review
中的反驳
](
pushback.md
)
参见
[
开发者指南
](
../developer/
)
, 为开发者提供了详细的Code Review指南。
review/reviewer/looking-for.md
浏览文件 @
a91b0915
# Code Review应该
注意
什么?
# Code Review应该
关注
什么?
注意:当我们考虑以下点时,应当始终遵循
[
Code Review标准
](
standard.md
)
。
...
...
@@ -53,7 +53,7 @@ Code Review中最重要的一个点就是把握住变更中的整体设计。变
如果变更改变了用户构建、测试、交互或者发布代码相关的逻辑,检测是否也更新了相关文档,包括READMEs, g3doc页面,以及任何相关文档。如果开发者删除或者弃用某些代码,考虑是否也应该删除相关文档。如果文档有确实,让开发者补充。
##
查看每一行代码
##
代码细节
查看你整个Code Review中的每一行代码,比如你可以扫到的数据文件,生成的代码或大型数据结构,但不要只扫一遍手写的类,函数或者代码块,然后假设它们都能正常运行。显然,有些代码需要仔细检查,这完全取决于你,但你至少应该
*理解*
所有的代码都在做什么。
...
...
@@ -61,14 +61,13 @@ Code Review中最重要的一个点就是把握住变更中的整体设计。变
如果你理解代码,但又觉得没有资格做代码评审,确保有资格的变更评审人员在代码评审时考虑到了安全性、并发性、可访问性、国际化等问题。
## Context
## 上下文
看改动上下文代码对Code Review很有帮助,因为通常Code Review工具只会显示改动部分上下几行代码,但有时你必须查看整个文件确保这次变更可以正常运行。例如,你可能看到加了4行代码,但是看到整个文件时才会发现这加的4行代码是在一个50多行的方法里,这个方法现在应该被拆分为多个小的方法了。
Code Review时考虑到整个系统的上下文也很重要,这次改动提升了系统健康度?或者增加了系统复杂性?少了测试用例?……
**不要通过哪些会损害系统健康的代码。**
很多系统变复杂都是因为一点一点的小改动日积月累造成的,所以千万不要放进来任何增加复杂性的改动。
##
不吝赞美 {#good_things}
##
亮点
如果你看到变更中做的好的地方,告诉开发者他们做的很棒,尤其是当他们用某种很精巧的方式解决你评论中提到的问题时更应不吝赞美。 Code Review过多关注于错误,但你也应该为开发者展现出好的一面点赞。在指导他人的时候,有时候告诉开发者正确的做法比告诉他们错误的做法更有价值。
## 总结
...
...
review/reviewer/navigate.md
浏览文件 @
a91b0915
# C
L指南
# C
ode Review步骤
## 主题
## 概要
现在你知道了
[
要从CL中得到什么
](
looking-for.md
)
,那能在众多文件中完成评审的方法是什么?
1.
这条改动是否
说的通?这改动是不是有一个好的
[
描述
](
../developer/cl-descriptions.md
)
?
2.
首先阅读
最重要的改动。总体上是不是一个好的设计
?
3.
按照合适的顺序阅读
省下的CL.
1.
这条改动是否
生效?这次变更是不是
[
描述
](
../developer/cl-descriptions.md
)
清晰
?
2.
首先阅读
CL最重要的一部分,整体上是否设计良好
?
3.
按照合适的顺序阅读
剩下的变更。
## 第一步: 全面了解
改动
## 第一步: 全面了解
变化
阅读
[
CL描述
](
../developer/cl-descriptions.md
)
并且明白CL大体内容。这些修改是否有意义?首先如果这个修改不应该有,请立刻说明为什么这些修改不应该有。当你因为这个拒绝了这次改动提交时,告诉开发人员该怎么去做是非常好的。
...
...
@@ -28,12 +27,12 @@
立即写下这些主要设计评注非常重要有两个主要原因:
-
开发人员通常会邮寄一个CL,然后在等待审核时立即基于该CL进行新工作。 如果您正在审查的CL中存在重大
设计问题,那么他们也将不得不重新设计其以后的CL。你能在他们做太多无用功之前制止他们。
-
开发人员通常会邮寄一个CL,然后在等待审核时立即基于该CL进行新工作。 如果您正在审查的CL中存在重大设计问题,那么他们也将不得不重新设计其以后的CL。你能在他们做太多无用功之前制止他们。
-
重要的设计变更比小的变更需要更长的时间。 开发人员几乎都有截止日期;为了在截止日期之前完成工作, 并在代码库中保留高质量的代码,开发人员需要尽快开始CL的所有主要的重做。
## 第三步:以适当的顺序
浏览
其余的CL
## 第三步:以适当的顺序
阅读
其余的CL
确认CL整体上没有大的设计问题后,请尝试找出逻辑顺序来浏览文件,同时还要确保不要错过对任何文件的审查。通常,在浏览了主要文件之后,按照代码审查工具向您展示它们的顺序浏览每个文件是最简单的。有时在阅读主要代码之前先阅读测试代码也是有帮助的,因为这样您就可以知道更改应该做什么。
下一节:
[
快速的code review
](
speed.md
)
下一节:
[
Code Review的速度
](
speed.md
)
review/reviewer/pushback.md
浏览文件 @
a91b0915
...
...
@@ -2,7 +2,7 @@
有时开发者会在Code Review中反驳你,他们可能不同意你的意见,或者抱怨你太严格了。
## 谁是
对的?
## 谁是
谁非?
当开发者不同意你的建议时,首先花点思考下他们是否是对的,但通常而言你比他们更熟悉代码,所以可能在某个方面理解更深。他们的争论有意义吗?从代码健康的角度来看他们的反驳有意义吗?如果是,让他们知道他们是对的,然后这个问题就解决了。
然而,开发者不总是对的,这种情况下评审者应当进一步介绍为什么他们的建议是对的。好的解释不仅得体现出对开发人员回复的理解,而且还要说明为什么要这么改。
...
...
@@ -18,9 +18,9 @@
## 稍后解决
一种常见的反驳原因是开发者希望能尽快完成任务。他们不想一轮又一轮地做Code Review,然后就会说他们会在后续
变更中处理这些问题,你只需要通过就行。有些开发者做的很好,他们会立马提交后续变更处理这些问题。然而,经验告诉我们原始变更通过之后拖的时间越久,就越不可能修复。除非开发者在当前变更通过后
*立马*
修复,否则他们就不可能修复。这并不是开发者不负责任,而是因为他们有好多工作要做,而修复工作也会因为工作压力而被遗忘。所以最好坚持让开发者
*现在*
就在变更
中处理掉这些问题,“留着以后清理”是一种不可取的方式。
一种常见的反驳原因是开发者希望能尽快完成任务。他们不想一轮又一轮地做Code Review,然后就会说他们会在后续
CL中处理这些问题,你只需要通过就行。有些开发者做的很好,他们会立马提交后续CL处理这些问题。然而,经验告诉我们原始CL通过之后拖的时间越久,就越不可能修复。除非开发者在当前CL通过后
*立马*
修复,否则他们就不可能修复。这并不是开发者不负责任,而是因为他们有好多工作要做,而修复工作也会因为工作压力而被遗忘。所以最好坚持让开发者
*现在*
就在CL
中处理掉这些问题,“留着以后清理”是一种不可取的方式。
如果
变更中引入了新的复杂性,提交之前必须清理掉,除非是
[
紧急情况
](
../emergencies.md
)
。 如果变更
中暴露出一些目前还无法定位的问题,开发者应该记录下这些bug,并将其分配给他们自己,确保这些问题不会被遗忘。他们还可以在代码中加入 TODO 注释,指向已经记录好的 bug。
如果
CL中引入了新的复杂性,提交之前必须清理掉,除非是
[
紧急情况
](
../emergencies.md
)
。 如果CL
中暴露出一些目前还无法定位的问题,开发者应该记录下这些bug,并将其分配给他们自己,确保这些问题不会被遗忘。他们还可以在代码中加入 TODO 注释,指向已经记录好的 bug。
## 抱怨太严格
...
...
review/reviewer/speed.md
浏览文件 @
a91b0915
# Code Review
s
的速度
# Code Review的速度
## 为什么Code Review
s
应该快?
## 为什么Code Review应该快?
**在谷歌内部,我们在持续优化团队开发新产品的速度**
,而不是优化单个开发人员编写代码的速度。 个人开发的速度固然重要,但它没有团队整体的速度那么重要。
如果
code r
eview速度慢了,可能会发生以下这些事:
如果
Code R
eview速度慢了,可能会发生以下这些事:
*
**整个团队的速度会降低**
,是的,你不快速响应别人的Code Review也可以完成其他的工作。但是,整个团队的新功能、bug修复可能会因为没有人做cr被延迟几天、几周甚至几个月。
*
**开发者应维护Code Review的流程**
,如果审查者很少回复Code Review,但是每次都对
变更提出大量的改动,这可能会打击到开发者。通常,开发者可能会抱怨审查者太严格。如果审查者能在开发者更新后快速响应,并提出有实质性提升的建议(能显著提升代码运行状况的变更
),抱怨就会消失。
**Code Review中绝大多数抱怨会随着CR速度的提升而消失。**
*
**可能影响到代码质量。**
如果CR慢了,可能会给开发者一种需要提交不太好代码的压力。CR慢了,也会影响到代码清理、重构、和现有
变更
的进一步提升。
*
**开发者应维护Code Review的流程**
,如果审查者很少回复Code Review,但是每次都对
CL提出大量的改动,这可能会打击到开发者。通常,开发者可能会抱怨审查者太严格。如果审查者能在开发者更新后快速响应,并提出有实质性提升的建议(能显著提升代码运行状况的CL
),抱怨就会消失。
**Code Review中绝大多数抱怨会随着CR速度的提升而消失。**
*
**可能影响到代码质量。**
如果CR慢了,可能会给开发者一种需要提交不太好代码的压力。CR慢了,也会影响到代码清理、重构、和现有
CL
的进一步提升。
## 应该以什么样的速度做Code Review?
...
...
@@ -42,11 +42,11 @@
除非另有明确说明,评审者应指明他们打算使用这些选项中的哪一个。
当开发者和评审者在不同时区时,应当着重考虑下通过CR,否则开发者还得等一天。
## 大的
变更
## 大的
CL
如果有人给你提交了一个非常大的Code Review,你也不确定你有时间看,你最好建议开发者
[
把
变更
拆分成几个小的部分
](
../developer/small-cls.md
)
,分多次Code Review,而不是一次性全部提交上来。这即便开发者多做一些额外的工作,也是可以把它拆分开的,而且拆分也有利于评审者。
如果有人给你提交了一个非常大的Code Review,你也不确定你有时间看,你最好建议开发者
[
把
CL
拆分成几个小的部分
](
../developer/small-cls.md
)
,分多次Code Review,而不是一次性全部提交上来。这即便开发者多做一些额外的工作,也是可以把它拆分开的,而且拆分也有利于评审者。
如果
变更
*不能*
拆分成多个小变更
,你也没有时间快速完整的Review整个代码,只是对整体设计提一些建议,然后发回给开发者改进。作为评审者,你的目标之一是在不牺牲代码质量的前提下,不阻碍开发者的进程或者尽可能让他们向前推进。
如果
CL
*不能*
拆分成多个小CL
,你也没有时间快速完整的Review整个代码,只是对整体设计提一些建议,然后发回给开发者改进。作为评审者,你的目标之一是在不牺牲代码质量的前提下,不阻碍开发者的进程或者尽可能让他们向前推进。
## 持续提升Code Review
...
...
@@ -55,7 +55,7 @@
## 紧急情况
当然也有一些紧急的
变更
需要快速走完这个Code Review流程,这时候在质量上的把控可以稍微放松一些。可以参考
[
紧急事件
](
../emergencies.md#what
)
一文来了解哪些是紧急事件哪些不是。
当然也有一些紧急的
CL
需要快速走完这个Code Review流程,这时候在质量上的把控可以稍微放松一些。可以参考
[
紧急事件
](
../emergencies.md#what
)
一文来了解哪些是紧急事件哪些不是。
下一篇:
[
如何在Code Review中写评论
](
comments.md
)
review/reviewer/standard.md
浏览文件 @
a91b0915
...
...
@@ -8,7 +8,7 @@ Code Review的主要目的是始终保证随着时间的推移,谷歌代码越
另一方面,保证代码库随时间推移越来越健康是审查者的责任,而不是让代码库质量变得越来越差。这很棘手,因为代码质量一般都会随着时间推移越来越差,尤其是在团队有明确时间限制、而且他们觉得不得不采取一些投机取巧的方式才能完成任务的情况下。
但是,代码评审者得对他们Review的代码负责,所以他们想始终确保代码库一致、可维护(其他指标见
[
"Code Review应该
看些
什么?"
](
looking-for.md
)
)
但是,代码评审者得对他们Review的代码负责,所以他们想始终确保代码库一致、可维护(其他指标见
[
"Code Review应该
关注
什么?"
](
looking-for.md
)
)
依据这些,我们将以下准则作为我们期望的Code Review标准:
...
...
@@ -24,7 +24,7 @@ Code Review的主要目的是始终保证随着时间的推移,谷歌代码越
注意:这篇文档中没有任何地方辩解在变更中的检查会让整个系统代码变得 _更糟糕_ 。你唯一能做的在
[
紧急度
](
../emergencies.md
)
中说明。
## 指导
## 指导
性
Code Review有个重要的作用,那就是可以教会开发者关于语言、框架或者通用软件设计原理。在Code Review中留下评论来帮助开发者学习新东西是很值得提倡的,毕竟共享知识也是长期提升系统代码健康度的一部分。但请注意,如果你的评论纯粹是教育性的,并且不是这篇文档中提到的关键标准,请在前面加上“Nit:”标识,或者明确指出不需要在这次变更中解决。
## 原则
...
...
@@ -40,4 +40,4 @@ Code Review有个重要的作用,那就是可以教会开发者关于语言、
如果这样都解决不了问题,那解决问题的方式就应该升级了。通常的方式是拉着团队一起讨论、让团队主管来权衡、参考代码维护者的意见,或者让管理层来决定。
**不要因为开发者和评审者不能达成一致而把变更一直放在那里。**
下一篇:
[
Code Review
应该看些
](
looking-for.md
)
下一篇:
[
Code Review
应该关注什么?
](
looking-for.md
)
编辑
预览
Markdown
is supported
0%
请重试
或
添加新附件
.
添加附件
取消
You are about to add
0
people
to the discussion. Proceed with caution.
先完成此消息的编辑!
取消
想要评论请
注册
或
登录