如何利用Github进行代码审查

HarGIKT 9年前

来自: https://realm.io/cn/news/codereview-howto/

当问到你们的团队是否进行代码审查这个问题时,大部分人的回答会是 ”项目进度跟的这么紧,哪还有时间进行代码审查啊?“。如果再加上频繁的人事变动和组织结构调整导致很多人对代码审查的可行性及意义产生了巨大的疑问。并且让团队的所有成员熟悉代码审查工具和流程本身也是一项费时费力的工作。那么,就让我们通过Realm团队的例子来看看他们是怎么进行代码审查的。

我的第一次代码审查经历

笔者回想起在当年在某韩企工作时,第一次进行代码审查的过程。

  1. 提出代码审查会议请求
  2. 与团队成员共享审查代码的范围,会议室,时间等信息。
  3. 在指定会议室进行代码审查之前分析并审查所有代码。

是的。这并不是一次成功的代码审查。因为大家都专注于自己的开发任务,根本就不主动提出代码审查请求。并且在进行代码审查会议之前,大家基本上对即将要审阅的代码一无所知。导致了会议的大部分时间浪费在对代码的说明上,并且大家也只能提出类似于拼写错误和命名规范这样简单的错误和改善建议。代码审查本身可以提高开发者的能力,让其从自身犯过的错误中学习,从他人的思路中学习。但是不当的流程可能会使代码审查的意义变的微乎其微。

使用代码审查工具

如果说上面所提及的代码审查方式是同步的代码审查,那么使用代码审查工具的结果就是异步的代码审查。代码审查工具在不对源代码造成任何修改的情况下,团队成员可以对指定代码进行批注和评论。为了进行高效的代码审查,我们可以使用的工具不仅有Gerrit, Review Board, Phabricator等这样的开源软件,而且对于一个预算比较充足的团队来说,JetBrains的 Upsource 和 Atlassian的 Crucible 都是不错的选择,可以在LDAP服务器上方便的搭建并且和JIRA缺陷管理工具,代码版本管理工具(git,svn)关联进行代码审查。利用这些工具来管理和追踪审查由于住团队更好的起步。下面是Upsource的官方示例。

使用Github的 Pull Request 进行代码审查

Github的 Pull Request 可以作为一个非常有用的代码审查工具。很多开发者了解[git flow](http://danielkummer.github.io/git-flow-cheatsheet/index.zh_CN.html)分支管理策略。但是由于它比较复杂,所以很难在短时间内使团队熟悉这套管理策略。相比于git flow,Github 提出更为简洁的 Github Flow 的分支管理策略,重点就是不直接对master分支进行提交或合并,而是通过提交 Pull Request,并且进行审阅和讨论后最终合并到master。 Github声称代码审查功能是Github的核心功能之一。

通过Pull Request @提及团队成员 让对方审阅自己的代码,指定成员跳转到指定分支后可以对代码进行评论,提出改善建议。并且帮助改善逻辑及缺陷。还可以通过触发器触发CI(持续集成服务器)自动进行集成测试。

那么让我们了解一下开发移动端数据库的Realm到底是怎么进行代码审查的?让我举一个 添加 isEmpty 功能的 Pull Request 作为例子说明,因为这个 Pull Request 已经被合并了,所以很多评论被显示成为‘outdated diff‘,如果想查看评论请点击’Show outdated diff’。

我们可以对指定代码进行提问及评论,回答并展开讨论。界面和我们熟知的社交网站也非常相似。

通过ci确保所有测试用例都通过测试,并且有2名以上的审阅人对代码进行审查后才能进行最终合并。在此之前,通过讨论和评论的方式对代码进行不断的重构和改善。

Git本身并不提供 Pull Request 功能。我们只能使用Github或者 Github Enterprise 提供的 Pull Request 功能进行代码审查。

还有Bitbucket 也提供类似的功能, 在Bitbucket中使用 Pull Request

Github官方提供的文档 关于 Pull Request 的说明 也可供我们参考。

是否应该一字不落的对所有的代码进行代码审查呢?

笔者认为应视情况而定,例如,当遇到一些紧急的UI更新,或者只有1名开发者的情况下进行代码审查显得有点不太合适。Realm的情况也差不多,关于产品的所有代码都会进行代码审查进行反复迭代,但是一些不重要的代码仓库(例如网页,示例代码等)就会视情况进行选择性的代码审查。在多名开发者同时开发一个项目的情况下,一些重要的的核心业务逻辑开发完成后,通过进行代码审查事先发现潜在的错误(查找逻辑上的瑕疵,写反了的布尔判断,潜在的对象的错误选择,类型不匹配,等等)从而提高代码的质量。

让代码审查成为一种习惯,一种文化。

让代码审查称为一种习惯,通过相互讨论和学习来提高自己的开发能力最好不过。想着我写的代码会让别人去审查,开发者会不由自主的更注重代码的可读性和逻辑性。相比强制性的进行代码审查,首先培养团队以产品和人为中心的开发文化显得更为重要。虽然说不容易但是值得一试。相比其他的各种福利,多数开发者更为看重公司的开发文化。即为了公司,也为了开发者自己更高效的开发。—————————————————————————————-

Realm是可以替代 SQLite和 Core Data 的移动端数据库
并且支持Swift了解详细信息

</div>