OpenStack 方式的代码变更审查

在几乎所有的代码仓库中,Gerrit 提供了 5 个选项来对补丁进行投票。+2 和 -2 选项仅供项目核心审查团队成员使用。每个人都可以使用 -1 和 +1 选项。你也可以在不投票的情况下留下评论 (0)。本文档是非详尽的指南,你可以使用其中的技术来审查变更,从而提供反馈,同时保持流程的顺畅。在所有情况下,请根据你的最佳判断使用,但我们提供了来自社区集体经验的启发式方法来指导你。

正是补丁提交者推动 OpenStack 的发展。作为审查者,请记住,你是在为提交者提供服务,而不是相反。作为提交者,请记住,每个人都受到相同规则的约束:即使对你的补丁进行投票的核心审查者也会将他们的变更通过相同的代码审查流程。

代码审查 +2

+2 投票仅供核心审查者使用。鼓励项目要求在合并变更之前需要两个 +2 投票,尽管有些项目只需要一个 +2 投票,并且许多项目在某些情况下放宽要求(例如,琐碎的变更)。如果你是核心审查者,请检查你的本地策略。令人困惑的是,两个 +1 并不等于 +2!

在未批准的情况下投票 +2 表示你希望另一位核心审查者批准该变更。如果另一位核心审查者已经投票 +2,那么你通常会在同一时间批准该变更。但是,你可能会推迟批准,以便作者或其他审查者有机会对一些琐碎的反馈做出回应,如果他们认为合适的话。如果反馈足够琐碎,这比仅投票 +1 更可取。

如果另一位核心审查者之前对较早的补丁集投票 +2,并且自那时以来补丁仅以你确信他们会喜欢的琐碎方式进行了更改,那么你可以用你自己的 +2 批准,而不是等待他们重新审查。当你这样做时,请留下评论解释你的决定。

代码审查 +1

对于非核心审查者,+1 表示你已经审查了该变更并且对其合并感到满意。仅留下 +1 而不留下评论除非你的审查历史为核心团队所熟知(在这种情况下,你很快就会加入它),否则用处不大。如果合适,请尝试留下评论 - 如果你测试了补丁,请说明;如果你不得不查找任何内容以确认它是正确的,请留下带有指向参考的链接的评论,以帮助下一个审查者。

核心审查者也可以使用 +1 投票。当你想让补丁没问题但你有一个未解决的问题,或者你有可以修复在 后续变更 中修复的次要反馈,但你想让贡献者有机会自己修复它 - 例如,因为你也想了解他们的意见,或者因为你试图帮助指导他们时,请考虑这样做。如果你不确定贡献者是否在寻找指导或更希望你修复补丁或自己提交后续变更,请尝试询问他们!许多贡献者会迅速响应来自核心审查者的 +1,因为他们知道这意味着 +2 即将来临,这比 -1 更有助于鼓励贡献者并保持善意。它还将帮助你更快地重新审查任何后续补丁集,因为你将看到你基本上已经同意了它,并且只需要检查任何差异。

即使作为核心审查者,你可能不熟悉项目的每个部分。如果你遇到一个你不太确定的区域的变更,你可以投票 +1,而你原本会投票 +2。与往常一样,留下评论说明原因总是值得的。

代码审查 0

如果你对变更没有强烈的看法,或者你只是需要回答一个问题才能形成意见,你可能会留下评论而不进行审查。除非你确信问题的答案会揭示一个问题,否则这比第一次投票 -1 更可取。没有 -1 投票的评论更容易被意外忽略,所以如果已经有一段时间没有得到答复,或者发布了新的补丁集而没有人回应你的评论,那么可能该尝试通过 IRC 联系提交者或考虑 -1 投票了。

代码审查 -1

使用 -1 审查来表明你发现了补丁中的重大问题,你强烈认为在合并变更之前应该纠正这些问题。有很多合法的理由这样做,从变更引入的新错误,到即使是提交消息中的一个拼写错误 - 如果(并且只有如果!)它是在一个关键单词中,这将使补丁难以在 git 历史记录中找到。再次使用你的最佳判断,但请记住,当你投票 -1 时,你正在阻碍他人的工作,因此请确保这是出于充分的理由,而不是可以通过其他方式解决的事情(例如,后续变更)。

还要记住,许多繁忙的审查者不会优先处理已经收到负面审查的变更,因此通过投票 -1,你不仅要求提交者进行另一个修订,你还在潜在地切断他们与其他反馈来源的联系。

-1 审查应始终伴有带有可操作反馈的评论。

如果你在补丁集的修订次数很多的情况下到达一个变更,请不要忘记查看之前的历史记录,如果你看到一些奇怪的东西。它可能已经被之前的审查者要求过。如果你是核心审查者,并且发现自己需要给出与另一位核心审查者给出的建议相反的建议,那么你有责任与另一位审查者达成一致,并最好让你们双方都记录下来。最好在投票 -1 之前,除非变更实际上破坏了某些东西(在这种情况下,留下评论表示你理解存在冲突的建议,并且你将尽快努力解决它)。处理核心审查者之间分歧的责任永远不在补丁提交者身上。

代码审查 -2

-2 投票仅供核心审查者使用。与其他投票不同,这个投票是“粘性的” - -2 投票即使提交者推送了新的(实质上不同的)补丁集也会保留。这意味着要删除 -2 投票需要来自同一核心审查者的操作,因此请小心。

-2 投票的目的是告诉提交者,他们花费在该变更上的任何进一步时间几乎肯定都会被浪费。如果你收到变更的 -2 审查,不要感到难过!审查者试图将你宝贵的时间和精力重新导向更有可能合并的变更。清晰的沟通意味着减少你所浪费的时间。

-2 审查应始终伴有评论,解释变更为何不符合项目目标,以便提交者可以理解原因并更有效地重新调整他们未来的贡献。

除了下面提到的“程序性 -2”之外,没有其他合法的 -2 投票用途。

程序性代码审查 -2

一些项目会在功能冻结之后和下一个发布分支之前,对功能变更放置 -2 投票,以确保在冻结期间不会意外合并任何功能。添加这些 -2 的人将在主分支对新功能开放后再次删除它们。他们应该留下评论解释正在发生的事情。提交者可以在冻结期间继续修改变更。

工作流 -1

工作流 -1 投票表示该变更当前尚未准备好进行全面审查。只有核心审查者和原始变更所有者可以投票工作流 -1。当提交更改的新的补丁集时,任何工作流投票都会被清除。这是获得对正在进行的工作的反馈比遗留方法(即隐藏不明确添加的变更)更好的方法。

核心审查者还可以使用工作流 -1 投票来防止在某些临时条件下合并变更,而不会中断代码审查过程。

后续变更

在可能的情况下,提交后续变更是解决次要问题而不通过要求另一个补丁集来暂停审查过程(从而擦除现有的审查)的好方法。只需 签出 现有的变更(使用 Gerrit 在“下载”下拉菜单中提供的命令;git review -d 命令;或 git-nit 工具),在顶部添加另一个提交,然后开始新的审查。

这通常比修改原始变更更好,前提是该变更实际上不会破坏任何东西。

修改变更

任何人都可以推送新的补丁集到现有的审查,有时这是解决问题的最佳方法。但是,请注意,这可能会让一些贡献者感到惊讶,甚至可能让他们觉得你试图窃取他们的补丁的功劳。事实并非如此 - 所有统计信息收集工具都将功劳归于变更的所有者(即,初始提交者)。如果你不认识提交者,最好留下评论说明你正在做什么(你可以将本节链接到项目团队指南作为解释的一部分)。确保使用 Gerrit UI签出 现有的补丁使用 Gerrit 在“下载”下拉菜单中提供的命令或 git review -d 命令,然后再使用 git commit --amend 合并你的修改,以便补丁作者字段保持不变,并且你仅在 Git 中列为提交者。如果你的修改是实质性的,你可以在提交消息中添加 Co-Authored-By 信用。

你可能想要修改现有变更的一些例子

  • 当提交者明确邀请你时

  • 当补丁需要重新基准时

  • 当提交者没有响应反馈一段时间时

  • 当你计划在明显的琐碎修改之后立即合并补丁时

  • 当你只需要修改提交消息(提交消息是不可变的,无法在 后续变更 中修复)

请注意,如果该变更不是系列中的最后一个(或唯一一个),则该系列中的其余部分也需要重新基准。在这种情况下,如果可能,最好让原始作者进行修改,因为将本地分支替换为 Gerrit 中的最新版本的过程可能需要对 Git 和 Gerrit 有相当深入的了解。