可评审代码之道

标签： 代码 | 发表时间：2013-03-21 22:17 | 作者：hitlion2008
出处：http://blog.csdn.net

如何组织改动的一些建议.

这篇文档是纯建议性的.Phabricator(一个源码审查工具)可以与很多源码控制策略一起使用,这篇文档中的建议产生的差异不会影响到把Phabricator作为代码审查和源码管理的使用.


概览

这篇文档描述了一个在Facebook和Phabricator中被成功的应用结构化改动的策略.大体上:
  • 每次提交应该尽可能的小,但不能再小了
  • 一次提交要小到是一个紧密结合的想法:不要让提交小到只有看到其他提交时才有意义.
  • 想法和提交应该是一一对应的:每次提交只实现一个想法而且各个想法应该用其各自的提交来实现.
  • 通过把大问题分解成小问题的方式来把大提交改成小提交,然后一次解决一个小问题.
  • 填写有合理的提交记录信息

众多的小提交

小巧简单的提交要比庞大复杂的提交要好的多.它们更易于理解,易于测试和易于评审.理解,测试和评审一个改动的复杂度比它的大小增加更快:每个改动做一件事的10个20行的改动通常比一个2000行做十件事的改动更容易理解.把一个做很多事情的大改动拆分成每个改动只做一件事的多个小的改动可以在完成同样目标的情况下降低整体的复杂度.

每次提交只做一件事.通常情况下,这意味着你应该在开发时应该把不同的改动分解成不同的提交.例如,如果你正在开发一个功能点同时遇到一个先前存在的Bug,应该暂存你的改动；签出一个干净的HEAD,用一个改动修复Bug；然后再把新功能代码衍合到修改Bug代码之上,这样就有了二个改动,每个都有一个原因("添加X新功能","修复Y Bug"), 而不是用一个改动带二个原因("添加X新功能和修复Y Bug").


(使用Git你可以在本地新功能分支上很容易的做,用命令git rebase, git rebase -i和git stash,或者使用git merge.使用Mercurial,可以使用书签和队列扩展.如果是SVN,虽然没有很多内置工具,但是可以使用多个工作复本.)

即使像整顿风格问题也最好要分解:因为它们要实现的是不同的目标.因为评审一个300行的"把tabs转成空格"加上一个30行的"实现Z功能点"要远比一个330行的"实现Z功能点和把tabs转成空格容易评审的多.


类似的,把相关的但复杂的改动分解成小巧且简单的部分.有一个夸张的比喻:如果你要建造一个新房子,不要用一个5000行的改动一下子把整个房子都盖好了.分成更小的更简单更容易理解的步骤:从地基开始,然后是框架等等.如果你决定用铁锹来挖地基和用硬纸板搭框架,如果决定是埋藏在5000行的内部设计和庭院设计, 那么它也会既容易迷失也难改正.每次仅做一点, 提供更大的问题能被理解的足够的环境.  但是每一步除必须外不做过多的事情,  以便能它让们独立存在.
一次改动的最小尺度是一个能独立工作的最小子问题的实现且要表达一个整个想法,不能是半个想法.你可随机选择行的方式来把一个1000行的改动机械的分成 10个100行的改动,但是没有一个是有意义的,这样做仅会增加整体的复杂度.正真正的目的是每一次改动有最小的复杂度,行数仅是一个经常与复杂度相关的代表而已.
我们通常会廷用Phabricator的做法.Phabricator的平均改动是35行.
到寻找关于评审大改动的信息

填写有意义的提交记录

关于这个话题Internet上有很多资源.所有的资源几乎都在表述同一个事情；这里也不例外.
唯一最重要的就是:提交记录信息应该 解释为何做这样的修改
Differential(Phabricator的一个工具)尝试去鼓励构建合理的提交记录,但仅能在结构上而不是内容上实施.
从结构上讲,提交信息可能是:
  • 标题,用一行简短描述改动
  • 概述,在细节上描述改动
  • 也许还有其他的.

内容远比结构重要.特别地,概述应该解释你为何做此修改并且为何选用那样的实现方式.改动本身通常能很好的解释改动的内容.例如,这明显是一个糟糕的提交记录信息:

fix a bug
但是这个也好不了哪去: 

Allow dots in usernames

Change the regexps so usernames can have dots in them.

这个比没有强点,但是仅说些能从diff中得到的信息.相反,你应该提供修改的上下文并且解释为什么如此修改,以及为什么这是正确的做法:

Allow dots in usernames to support Google and LDAP auth

To prevent nonsense, usernames are currently restricted to A-Z0-9. Now that
we have Google and LDAP auth, a couple of installs want to allow "." too
since they have schemes like "[email protected]" (see Tnnn). There
are no technical reasons not to do this, so I opened up the regexps a bit.

We could mostly open this up more but I figured I'd wait until someone asks
before allowing "ke$ha", etc., because I personally find such names
distasteful and offensive.
这个信息不能从改动本身获取.并且对评审者以及事后试图理解改动的人来说也更有用.

一个简单的解释为什么的方法是参考促使你做改动的东西(如Bug,问题,版本等).
Differential也包含一个默认情况下必须的项:Test Plan.在 Differential用户指南上有关于此荐的详细描述.可以在配置中把它置为可选或者禁掉,但是建议采用它.这条信息会对评审者特别有用.
在提交记录中还有一些有时感觉很强烈但又不是那么重要的东西:
  • 是否要以及在哪里换行
  • 标题的最大长度
  • 标题后是否应有句点
  • 用时态或语态,如"fix/add"还是"fixes/adds".
  • 提交记录中一些其他的与"为什么此改动会出现"不相干的东西
  • 虽然也许拼写和语法不是那么十分的火糟糕?

Phabricator关于此没有指南.如果喜欢也诚然可以为你的组织设立指南,但是将 "为什么"放进提交记录是最重要的一部分.

原文: Writing Reviewable Code

作者：hitlion2008 发表于2013-3-21 22:17:01 原文链接
阅读：86 评论：0 查看评论

相关 [代码] 推荐：

代码重构

- - ITeye博客
随着程序的演化，我们有必要重新思考早先的决策，并重写部分代码. 代码需要演化；它不是静态的事物. 重写、重做和重新架构代码合起来，称为重构.    当你遇到绊脚石  ---  代码不在合适，你注意到有两样东西其实应该合并或是其他任何对你来说是"错误"的东西  -------- . 如果代码具备以下特征，你都应该考虑重构代码：.

代码小比较

- Tim - 斯巴达第二季
判断上百万个4k的buffer是否为全0，我最先想到的办法是：zero_buffer = malloc(4096);. /* 循环百万次读取buffer */.         /* 全0 */. 由于好奇，看看shell工具cp的代码，它的解决办法是：. /* 循环百万次读取buffer */.         /* 全0 */.

两行 JavaScript 代码

- MessyCS - Dreamer's Blog
最近看到了两行 JavaScript 代码，很受启发. 在 JavaScript 中，我们可以获取HTML元素的属性值，例如 element.id. 但是，因为 for 和 class 是 JavaScript 中的关键字，所以在 JavaScript 中这两个属性名称分别用 htmlFor 和 className 代替，于是在封装的时候需要先对这两个属性进行特殊判断.

Netty代码分析

- LightingMan - 淘宝JAVA中间件团队博客
Netty提供异步的、事件驱动的网络应用程序框架和工具，用以快速开发高性能、高可靠性的网络服务器和客户端程序[官方定义]，整体来看其包含了以下内容：1.提供了丰富的协议编解码支持，2.实现自有的buffer系统，减少复制所带来的消耗，3.整套channel的实现，4.基于事件的过程流转以及完整的网络事件响应与扩展，5.丰富的example.

python代码调试

- - 阿里古古
【转自： http://blog.csdn.net/luckeryin/article/details/4477233】. 本文讨论在没有方便的IDE工具可用的情况下，使用pdb调试python程序. 例如，有模拟税收计算的程序：. debug_demo函数计算4500的入账所需的税收. 在需要插入断点的地方，加入红色部分代码：如果_DEBUG值为True，则在该处开始调试（加入_DEBUG的原因是为了方便打开/关闭调试）.

ios代码开源

- - CSDN博客移动开发推荐文章
本人从10年开始搞ios开发,从菜鸟到现在的入门,期间遇到了许多困难,也总结了一些东西,本着开源精神,希望大家共同成长的目的把这个工程开源出来.. 这个工程是从11年到13年之前完成的.主要是我平时用到的一些基础功能模块.其中有其他开源的代码和我自己写的一些.代码结构基本乱,12年以后的代码结构还可以,不是很乱,之前水平有限,如果不怎么样就别喷我了.

Oracle错误代码

- - 数据库 - ITeye博客
ORA-00001: 违反唯一约束条件 (.). ORA-00017: 请求会话以设置跟踪事件. ORA-00018: 超出最大会话数. ORA-00019: 超出最大会话许可数. ORA-00020: 超出最大进程数 (). ORA-00021: 会话附属于其它某些进程；无法转换会话. ORA-00022: 无效的会话 ID；访问被拒绝.

Java代码优化

- - ImportNew
2016年3月修改，结合自己的工作和平时学习的体验重新谈一下为什么要进行代码优化. 在修改之前，我的说法是这样的：. 就像鲸鱼吃虾米一样，也许吃一个两个虾米对于鲸鱼来说作用不大，但是吃的虾米多了，鲸鱼自然饱了. 代码优化一样，也许一个两个的优化，对于提升代码的运行效率意义不大，但是只要处处都能注意代码优化，总体来说对于提升代码的运行效率就很有用了.

用 pylint， 写好代码

- Nickcheng - 赖勇浩的编程私伙局
赖勇浩（http://laiyonghao.com）. Pylint 是一个 Python 代码分析工具，它分析 Python 代码中的错误，查找不符合代码风格标准（Pylint 默认使用的代码风格是 PEP 8）和有潜在问题的代码. Pylint 是一个 Python 工具，除了平常代码分析工具的作用之外，它提供了更多的功能：如检查一行代码的长度，变量名是否符合命名标准，一个声明过的接口是否被真正实现等等.

完美的代码——Programmers（24）

- 山石 - FeedzShare
来自: 西乔的九卦 - FeedzShare  . 发布时间:2011年06月02日,  已有 2 人推荐. 慢工出细活，只要你要求快，需求分析之类的步骤都只能是过长而已. 载于《程序员》杂志2011年第4期. 这个系列的漫画讲述程序员——这种神秘人类的囧事，故事多来源于我身边的程序员朋友,且以互联网开发背景为主.