这问题跟 code review 有个毛线关系，分明是写测试的问题，修 bug 的时候怎么不写相关测试代码……

顶LS。

#1 楼 @aptx4869 谢谢，首先我承认测试可以解决这个问题，测试也要写，我只是拿这个问题举例子（可能举得不太好....... ：-(.......），解决这个问题的办法有很多，我想问的不是怎么解决它，而是怎么做 code review，请大神介绍下，谢谢！

#1 楼 @aptx4869 +1

@aptx4869 @raven @mouse_lin 重新编辑了下帖子，突出下重点，谢谢！

#3 楼 @Blues 如果你们没有一个强悍的LEADER来做这个，那么就要求大家互相看，并且需要把 code review 的时间计算入工作时间，这样大家做起来才不会有压力。code review和测试是挺密切的，你所 review 的代码肯定要包含测试，因为测试本身也是代码。。

#6 楼 @raven 谢谢，对于 rails 我们确实没有一个非常强的 rails leader，把 code review 计入工作时间没有问题。我说的给他们“压力“，是指他们的代码现在是有人看的，写的不好大家是有目共睹的，是指的这个压力，测试的代码我已经要求 rails 的 developer 做了，我检查了下，这个已经确保是有的，只不过我目前保证不了测试是全面的。

我的意思是我会单独拿出一天或者半天的时间去做这个事情，让他们就在那个时候互相看还是提前给时间让他们看，然后 review 的当时写测试代码？

或者是我们拿出一段觉得可以优化的代码，然所有人一起讨论？

#7 楼 @Blues 我的经验是 例如小组里有ABC三个人，A的一个 PR(pull request) 做好了。那么就叫 b 和C去看，然后 b 和C至少有一个人去 review 然后直接在 PR 里评论（我们使用 github），其它人都会看到这个评论 如果有意见也可以一起来。这个不是固定到哪一天去做，而是有需要的时候去做。

一个 PR 想要 merge 最基本两点就是 reviewed and CI passed.

@raven 非常感谢！我以为需要拿出一整天的时间单独做这个事情，现在看来，这是在准备 deploy 的的一项基本的工作。有点明白了，谢谢！

#9 楼 @Blues 严格控制 production 分支的质量 :p

#10 楼 @raven 嗯。Thanks！

#8 楼 @raven 我记得 teahour 的采访里讲，他们团队是互相看的，一个人的代码要另外两个人看。但是那些都是形式。

讲讲我的思路： 首先，业务逻辑的 code，需求 ticket 要拆分成技术点，每个技术点如何实现写到 issue 里，这样，写的人和分配到 issue 的人，都清楚要干什么。 然后就是按照技术点去 review 是否符合要求。这个工作很耗时但是从团队协作和质量控制上，这个时间是要投入的。 至于测试，这个可以有专人维护（如果人力够的话，我是建议专人维护的，这样可以保证测试用例的连贯和测试覆盖程度高），如果个人维护，review 先从测试开始，再看代码，方便理解如何实现的。

review 测试我更看重的是逻辑覆盖程度。当然，具体情况具体分析。

如果是 rails view 的改动，因为单元测试意义不大，view 的测试在回归测试时重点检查。

我非常看重 code review 这个环节，可以让团队更行动一致。也可以阻挡一下需求对 code 的直接影响。毕竟太多人的坏习惯是跟着需求做，最后一团乱麻。当然啦，赶工的除外。。。。赶工的东西一定要找时间重构的。

code review 包括两种，一个是高级评审，一个同行评审 高级评审是高手对菜鸟的，一般由技术经理来进行，重点是看这个工程师写的代码是否合格。 同行评审是技术人员之间的，重点是代码的交流，代码互通，风格互通。 当然评审本身的目标，找 bug、提高代码质量还是不少的

#13 楼 @ZombieCoder 我觉得，对于评审的人也是自身学习的过程，协同开发，每个人都要从别人那了解各自的思路，思路太独特，就要用业务流程和设计规范来约束下。 我认为 manager 的很重要的事情就是设计开发思路。约束开发行为。

#12 楼 @liwei78 谢谢！有一点我不太明白的是：技术点需要写到 issue 里面去？另外对于“按照技术点去 review 是否符合要求”，受教了，我一般 review 都是通过应用表现出现的功能是否符合要求，以前没太管技术点的实现情况。 @ZombieCoder 看来对于 rails 的 review 我们团队目前适合同行评审。谢谢！

#15 楼 @Blues 举个例子，书上的例子。

一天，客户说，我们订书的系统，对于 VIP 用户，订购满 30 元免运费。快加上这个功能，我们明天用。

那么技术团队怎么应战？？？？

首先这个需求，PM 必须拆解，比如 1，我们改动那个 model，2，我们改动那个 controller 的那个方法，增加哪个判断，3，这次改动对应的流程有调整，按照 xxx 流程图补充测试，4，我要跟客户确认：如果它买了一个冰箱，再买了 1 本书，还免运费么？

至于代码细节大家也都编程，或许都能想到了，不过像 4 里面的问题，如果在 coding 的时候才发现，再去找客户，恐怕就来不及了。还有，对于一个不了解需求的“新”人或者刚睡醒的“老”人，拿到拆分好的 issue 是很幸福的，完成快，测试人员也不会打回 issue。

要知道在华为，issue 被退回两次是要被调岗或者停职的。。。。。。

http://yedingding.com/2013/08/08/dig-into-code-review-process.html

#17 楼 @yedingding 这是敏捷开发很好的总结。当然不敏捷，又如何，毕竟代码能正确工作，谁还能说它什么呢。

#18 楼 @liwei78 具型的可以听 Terry 在 Teahour 讲的，我们团队就是那样子做 review 的

#16 楼 @liwei78 例子很直观，谢谢！issue 被退回 2 次就调岗或者停止，这叫我情何以堪呐。。。 @yedingding 谢谢！快读看了下，我想问的是介不介意我把你的这篇文章拿出来讨论一下，然后按照我们的情况适当修改一下？

#18 楼 @liwei78 参考<程序员的自我修养>

#19 楼 @yedingding 听过的，很受用。是这期：http://teahour.fm/2013/10/21/talking-remote-work-with-allen-wei.html。想远程工作的话必听。

#22 楼 @liwei78 喔，是远程工作那一期，#35 吧。Kevin 和 Terry 采访 Allen

#18 楼 @liwei78 @yedingding 谢谢！ @liwei78 很多时候程序员们都会有些“癖好”，这一般都是高手的习惯，他们会认为代码能正确工作并不是 100% Ok，而是让他觉得所有代码他 (他们那一类人) 看来都是 Ok 的才是真的 Ok，而他们把这些“癖好”，汇编成一些基本的规范让人们去学习，不可否认的是往往 (不一定是 100%) 他们说的是对的，对于项目来说是有利的。 @yedingding 只听过，没看过这本书。我内心认为我上面的理解就是这本书所要讲述的问题。不过有有时间我还是会读一下，来确认一下我的看法，哈哈！

#24 楼 @Blues 嗯，精神洁癖，我们团队很严重。哈，我没看过，我只是想起了星爷的<演员的自我修养>

#24 楼 @Blues 这种有决定意义的人很重要的，团队里非常需要这种人，有点小问题也就无所谓了。呵呵。。 #25 楼 @yedingding 我也是精神洁癖，我受不了没有流程图，没有测试的东西摆在我面前。我倾向于小而精的产品。

#26 楼 @liwei78 我们从不画流程图，我们不追求 100% 覆盖率

#27 楼 @yedingding 那你们怎么整理需求和把需求变成 code 的呢？？？小伙伴们一起来偷窥下。。哈哈哈

#26 楼 @liwei78 对啊，这个人确实关键，如果找不对这样的人，对于团队来说是个灾难啊，所以目前最好的办法就是自己成长为那样的人。流程图感觉不是那么重要，一般都是给出一个 mockup，然后大家就知道流程了，感觉 mockup 就替代了流程图了。 @yedingding 个人认为 100% 的覆盖率过于困难了，但是一个项目长久下去，其实还是在朝着 100% 的覆盖率前进，只是它是一个缓慢的过程而已。

#28 楼 @liwei78 用 Pragmatic.ly 啊，为什么需要流程图？变成 code 的话，来 RubyConf 听我们团队的 Roy 讲《Discovering Better Object Oriented Design with Tests》

#29 楼 @Blues 是的，我的想法是有点进击。。。不过我之前的经验就是，流程要落到纸上，这样大家都有迹可循。定下来的东西就不要轻易改。这是 11 年一个项目的教训。

覆盖率 100% 是不可能的，越到后期越不可能，不过核心逻辑还是要 100% 的。if...else..多了真是灾难。

重构的基础是测试，不接近 100% 测试的重构有风险。。

#29 楼 @Blues 嗯，Mockup 或者 Feature

#30 楼 @yedingding 好的，一定会有收获。。

#33 楼 @liwei78 哈，明晚见，:)

#34 楼 @yedingding 明晚 9 点到北京，进二环也得 10 点以后了，哈哈哈哈。你们先玩吧。后天我早上去会场。

#31 楼 @liwei78 任何时候有了教训之后得到的经验一般是很难改变的，我理解这种情况，我自己也会这样，但是后来回过头，仔细想想，“好像是我把承担责任的对象搞错了，根本不是那个的错，如果怎么怎么做就可以避免这个问题了”。

#36 楼 @Blues 我更倾向用“公约”的形式做事。比如，说过的事情是有文档可查的，确认的逻辑有文档描述，如果及其复杂，那么，需要很大的耐心来讲解其中的关节之处。这样能做到透明。。。。当然啦，这样做事很累心。。

#37 楼 @liwei78 根据团队规模和公司的规定不同，其实只是一种习惯。成规模的公司大部分都是你说的这样的，做事情一定要有依据，小团队可能不会这样，一个是时间因素，你知道，那是很累心的，另一个是必要性，极端例子，如果团队只有一个人开发，除非后来有大批招人的计划，否则那根本就没必要。

#38 楼 @Blues 是的，协作的时候每个人观点都不同，如果不控制，那么必然引起矛盾。控制的方法我倒不建议自我约束，我更倾向强制约束，也就是按照敏捷要求走，那么敏捷也有很多方式，那就用一种大家都认同的方式。 在重构项目时，约束更重要。

“原因就是修改 bug 的代码被后来 push 的代码覆盖了”

这是怎么做到的

#40 楼 @sdpfoue 话说我不是已经编辑了主题了么？为什么还可以看得到？

就是一个 master，然后 branch A，branch B，branch A fix bug，然后 branch B merge，conflict 中，直接采用了 branch B 里面的代码，把 branch A 里面的修改 bug 的代码直接去掉了。所以就.......

#41 楼 @Blues 这不是 100% B 的问题吗。有冲突不看代码就抛一支。话说这种人我也碰到过一次还死不认账，在 git 面前都是徒劳

#42 楼 @sdpfoue 对啊，是 branch B 的问题，但是由于沟通不够，branch B 不知道那些跟他不同的代码是干什么的，然后也没有人进行 review，所以才出的问题，所以我觉得@raven的办法很好。