草庐IT

开源代码评审的十个通用步骤

Martin Kopec 2023-03-28 原文

你是否需要在你还没有完全理解整个项目时就对代码进行评审?抑或你避开了评审,以免让你看起来不知道如何进行。

本篇文章想要告诉你一个更好的方法。代码评审code review

我还记得作为实习生加入 红帽Red Hat

如果你对一处改动投了 +1,而别人投了 -1,这又意味着什么呢?答案是不意味任何事!你可能只是漏掉了一处别人注意到的细节。这不意味着世界末日。这也是为什么我们会用投票系统。正如同所有开源项目一样,代码合并是一项协同工作。

最近,我接到了太多的代码评审工作,以至于我几乎做不过来。我同时也注意到,参与评审的贡献者数量正在稳步减少。

出于这个原因,我想要写一篇文章阐述我对代码评审的个人观点。在这篇文章里,我会分享一些诀窍与技巧。我将会向你展示几个用来问自己的问题,以及在评审代码时需要注意的一些地方。

代码评审的目的是什么?

你是否曾写过一个非常简单的补丁?你认为它是如此微不足道,不需要审查。或许你直接就合并了它。直到晚些时候,你意识到你犯了个错误,一个明显的或是愚蠢的错误,比如错误的缩进,比如几行重复的代码而不是调用函数(是的,这些都是经验之谈!)。

如果有其他人来审查代码,就会发现这些东西。

代码评审的一个目的便是为你带来一双新的眼睛,从新的视角看待你要尝试解决的问题。这种新的背景也正是为什么代码评审至关重要。

你可能认为你必须是一个语言专家,才能审查别人的代码、项目,或两者。让我来告诉你一个所有代码评审者都想跟你说的秘密吧:大错特错!你并不需要完全理解该项目或者编程语言,就可以为一个改动提供全新的视角。下面,我将向你展示代码评审的通用流程。

代码评审的通用流程

这是我的代码评审流程,拆分成了几个要点。这个流程包含了我会问自己的一些问题,以帮助我专注于代码的变化以及其后果。你不需要严格依照这个顺序来进行评审。如果有任何原因导致你无法执行其中的某一步,跳过那一步就好。

1、理解改动,它想要解决的问题,以及为什么要这么做

为什么需要改动的解释以及任何相关背景都应该被放在 提交commit

改动想解决的问题需要被解决吗?它是项目应当关注的问题,还是与项目完全无关?

2、你会如何实现解决方案?它会不一样吗?

在这个时候,你应该已经知道代码改动是为了什么。换做是你会怎么做?在进一步对改动进行细节评审前,先思考这个问题。如果你想出了一个不一样的解决方案,并且你认为你的方案更好,在评审中提出来。你不需要投 -1;去问问作者为什么没有往那个方向走,看看这次讨论会把你们带向何方。

3、运行有改动和没有改动的代码

我通常会在代码中设置几个断点,运行代码并检查新代码是如何与其余部分互动的。

如果你无法运行整个代码,试着将带有新代码的函数复制到一个新的本地文件,模拟输入数据,然后运行。这在你不知道怎么运行整个项目,或者无法接触到运行所需的特殊环境时很有帮助。

4、新代码会破坏任何东西吗?

我是说,任何东西。想一想可能的后果。

以一个新的命令行选项为例,它会总是被目标所接受吗?

是否存在这样一种情况,使得新选项无法被接受或是会与其他东西起冲突?

或许新代码是导入了新的东西。那么这个新的库,以及可能的新的依赖关系,能够在老版本或者项目的运行系统中被找到吗?

安全方面呢?新的依赖足够安全吗?你至少可以在网上快速地搜索一下。还有,注意一下控制台日志里的警告。有的时候在同一个库里也可以找到更安全的函数。

5、新代码是否有效?

你刚刚确认了被提出的解决方案大概是正确的。现在该检查代码本身了。你需要关注代码的有效性和必要性。

检查新代码的风格。它与项目的代码风格相匹配吗?任何开源项目都(应该)有一份文档告知(新)贡献者项目所遵循的风格和优秀实践。

比如说,OpenStack 社区的所有项目都有一份 HACKING.rst 文件。你经常也能找到一份​​新贡献者指南​​包含所有必须知道的信息。

6、确认所有新增的变量和导入都被使用

你正在评审的代码常常已经过多次迭代,有的时候代码的最终版本与初始版已迥然不同。所以我们很容易忘记一些在历史版本中加入的变量与引用。自动化检测通常会用到 lint 工具,类似 Python 中的 [flake8][12]。

(LCTT 译注:​​lint​​ 指编程中用来发现代码潜在错误和约束代码风格的工具,起源于 C 语言编程中的静态分析工具 ​​lint​​。“lint” 本意为衣服上积累的绒毛与灰尘,“lint” 的取名寓意则在于捕捉编程时产生的“绒毛与灰尘”)

(LCTT 校注:我建议,“Lint” 工具可以翻译为 “代码清理” 或 “代码清洁” 工具。)

你可以在不声明新变量的情况下重写代码吗?通常情况下你可以,但问题是这样是否更好。这会带来什么益处吗?我们的目标不是要创造尽可能多的单行代码,而是写出高效且易读的代码。

7、新的函数和方法是否必要?

项目里的别的地方是否存在可以被复用的功能类似的函数?确保避免重新发明轮子以及重新实现已经被定义的逻辑永远都是值得的。

8、有单元测试吗?

如果补丁增加了新的函数或者在函数内添加了新的逻辑,它也应该附带对应的单元测试。新函数的作者总是比别人更适合写该函数的单元测试。

9. 验证重构

如果这次提交对现有代码进行了重构(它可能重命名了某个变量,或者是改变了的变量的作用域,或者是通过加减参数来改变函数的足迹,又或者是删去了某个东西),问一问你自己:

  • 这个可以被删除吗?它会影响到稳定分支吗?
  • 所有出现的地方都删掉了吗?
你可以利用 ​​grep 命令​​ 来查找。你不会相信有多少次我投 -1 就是因为这个。这是一个任何人都会犯的简单错误,也正因如此任何人都可以发现它。

提交的所有者很容易忽略这些事情,这完全可以理解。我也犯过很多次这种错误。我最终发现问题的根源在于我太急于提出评审,以至于我忘记了对仓库进行整体检查。

除了对项目仓库的检查外,检查其他代码用户也十分必要。如果有别的项目导入了这个项目,它们可能也需要进行重构。在 OpenStack 社区中,我们有对应的工具来查询别的社区项目。

10、项目文档是否需要做出更改?

你可以再一次使用 ​​grep 命令​​ 来检查在项目文档中是否提到了相关的代码改动。用常识来判断这次改动是否需要被收入文档以告知最终用户,还是只是一个不会影响用户体验的内部变化。

额外提示:考虑周到

当你在评审完新代码后提出建议或评论时,要考虑周到,反馈准确,描述详尽。如果有你不理解的地方就发出提问。如果你认为代码存在错误,解释你的理由。记住,如果作者不知道什么地方出了问题,他们就无法修复它。

最后几句

唯一的坏评审是没有评审。通过评审和投票,你提供了你的观点并为此投票。没有人指望你来做出最终决定(除非你是核心维护者),但是投票系统允许你提供你的观点和意见。相信我,补丁所有者会很高兴你这么做了的。

有关开源代码评审的十个通用步骤的更多相关文章

  1. ruby - 如何在 buildr 项目中使用 Ruby 代码? - 2

    如何在buildr项目中使用Ruby?我在很多不同的项目中使用过Ruby、JRuby、Java和Clojure。我目前正在使用我的标准Ruby开发一个模拟应用程序,我想尝试使用Clojure后端(我确实喜欢功能代码)以及JRubygui和测试套件。我还可以看到在未来的不同项目中使用Scala作为后端。我想我要为我的项目尝试一下buildr(http://buildr.apache.org/),但我注意到buildr似乎没有设置为在项目中使用JRuby代码本身!这看起来有点傻,因为该工具旨在统一通用的JVM语言并且是在ruby中构建的。除了将输出的jar包含在一个独特的、仅限ruby​​

  2. ruby-on-rails - Rails 源代码 : initialize hash in a weird way? - 2

    在rails源中:https://github.com/rails/rails/blob/master/activesupport/lib/active_support/lazy_load_hooks.rb可以看到以下内容@load_hooks=Hash.new{|h,k|h[k]=[]}在IRB中,它只是初始化一个空哈希。和做有什么区别@load_hooks=Hash.new 最佳答案 查看rubydocumentationforHashnew→new_hashclicktotogglesourcenew(obj)→new_has

  3. ruby-on-rails - 浏览 Ruby 源代码 - 2

    我的主要目标是能够完全理解我正在使用的库/gem。我尝试在Github上从头到尾阅读源代码,但这真的很难。我认为更有趣、更温和的踏脚石就是在使用时阅读每个库/gem方法的源代码。例如,我想知道RubyonRails中的redirect_to方法是如何工作的:如何查找redirect_to方法的源代码?我知道在pry中我可以执行类似show-methodmethod的操作,但我如何才能对Rails框架中的方法执行此操作?您对我如何更好地理解Gem及其API有什么建议吗?仅仅阅读源代码似乎真的很难,尤其是对于框架。谢谢! 最佳答案 Ru

  4. ruby - 模块嵌套代码风格偏好 - 2

    我的假设是moduleAmoduleBendend和moduleA::Bend是一样的。我能够从thisblog找到解决方案,thisSOthread和andthisSOthread.为什么以及什么时候应该更喜欢紧凑语法A::B而不是另一个,因为它显然有一个缺点?我有一种直觉,它可能与性能有关,因为在更多命名空间中查找常量需要更多计算。但是我无法通过对普通类进行基准测试来验证这一点。 最佳答案 这两种写作方法经常被混淆。首先要说的是,据我所知,没有可衡量的性能差异。(在下面的书面示例中不断查找)最明显的区别,可能也是最著名的,是你的

  5. ruby - 寻找通过阅读代码确定编程语言的ruby gem? - 2

    几个月前,我读了一篇关于ruby​​gem的博客文章,它可以通过阅读代码本身来确定编程语言。对于我的生活,我不记得博客或gem的名称。谷歌搜索“ruby编程语言猜测”及其变体也无济于事。有人碰巧知道相关gem的名称吗? 最佳答案 是这个吗:http://github.com/chrislo/sourceclassifier/tree/master 关于ruby-寻找通过阅读代码确定编程语言的rubygem?,我们在StackOverflow上找到一个类似的问题:

  6. ruby - Net::HTTP 获取源代码和状态 - 2

    我目前正在使用以下方法获取页面的源代码:Net::HTTP.get(URI.parse(page.url))我还想获取HTTP状态,而无需发出第二个请求。有没有办法用另一种方法做到这一点?我一直在查看文档,但似乎找不到我要找的东西。 最佳答案 在我看来,除非您需要一些真正的低级访问或控制,否则最好使用Ruby的内置Open::URI模块:require'open-uri'io=open('http://www.example.org/')#=>#body=io.read[0,50]#=>"["200","OK"]io.base_ur

  7. 程序员如何提高代码能力? - 2

    前言作为一名程序员,自己的本质工作就是做程序开发,那么程序开发的时候最直接的体现就是代码,检验一个程序员技术水平的一个核心环节就是开发时候的代码能力。众所周知,程序开发的水平提升是一个循序渐进的过程,每一位程序员都是从“菜鸟”变成“大神”的,所以程序员在程序开发过程中的代码能力也是根据平时开发中的业务实践来积累和提升的。提高代码能力核心要素程序员要想提高自身代码能力,尤其是新晋程序员的代码能力有很大的提升空间的时候,需要针对性的去提高自己的代码能力。提高代码能力其实有几个比较关键的点,只要把握住这些方面,就能很好的、快速的提高自己的一部分代码能力。1、多去阅读开源项目,如有机会可以亲自参与开源

  8. 7个大一C语言必学的程序 / C语言经典代码大全 - 2

    嗨~大家好,这里是可莉!今天给大家带来的是7个C语言的经典基础代码~那一起往下看下去把【程序一】打印100到200之间的素数#includeintmain(){ inti; for(i=100;i 【程序二】输出乘法口诀表#includeintmain(){inti;for(i=1;i 【程序三】判断1000年---2000年之间的闰年#includeintmain(){intyear;for(year=1000;year 【程序四】给定两个整形变量的值,将两个值的内容进行交换。这里提供两种方法来进行交换,第一种为创建临时变量来进行交换,第二种是不创建临时变量而直接进行交换。1.创建临时变量来

  9. git使用常见问题(提交代码,合并冲突) - 2

    文章目录git常用命令(简介,详细参数往下看)Git提交代码步骤gitpullgitstatusgitaddgitcommitgitpushgit代码冲突合并问题方法一:放弃本地代码方法二:合并代码常用命令以及详细参数gitadd将文件添加到仓库:gitdiff比较文件异同gitlog查看历史记录gitreset代码回滚版本库相关操作远程仓库相关操作分支相关操作创建分支查看分支:gitbranch合并分支:gitmerge删除分支:gitbranch-ddev查看分支合并图:gitlog–graph–pretty=oneline–abbrev-commit撤消某次提交git用户名密码相关配置g

  10. ruby - 这两段代码有什么区别? - 2

    打印1:defsum(i)i=i+[2]end$x=[1]sum($x)print$x打印12:defsum(i)i.push(2)end$x=[1]sum($x)print$x后者是修改全局变量$x。为什么它在第二个例子中被修改而不是在第一个例子中?类Array的任何方法(不仅是push)都会发生这种情况吗? 最佳答案 变量范围在这里无关紧要。在第一段代码中,您仅使用赋值运算符=为变量i赋值,而在第二段代码中,您正在修改$x(也称为i)使用破坏性方法push。赋值从不修改任何对象。它只是提供一个名称来引用一个对象。方法要么是破坏性

随机推荐