关于 CodeReview，和团队小伙伴产生了分歧。

[niceyuri]

背景

最近团队正在用 Java 重构 PHP 老项目（底层服务），陈年老代码，难度很大。因为测试资源有限，所以在开发时对于质量的标准就很严格。 基于此，团队达成了共识：要花点时间写单元测试和进行严格的 CodeReview 。

情况

在执行过程中呢，关于具体的 CodeReview 和单元测试的实践上，我和团队小伙伴产生了分歧。我理解的 CodeReview 是应该少量多次，每天提交代码时互相 Review 一下，人也轻松。而一些团队小伙伴认为，在提测之前，进行一次大而全的 Review 就可以了。包括对待单元测试的态度也差不多。 关键的点在于，我的个人想法是：如果不能频繁地做 Review ，那么等到最后，可能会迫于交付的压力，对着写了好几个月的代码望洋兴叹，最后就是应付交差，后期改问题的时间也会很长。而小伙伴们似乎对这个悲观预期不是很理解，我也无法说服他们。

结果

在一番讨论后，我们勉强达成了个共识，以模块开发完成为节点进行 Review ，也就是在数月的开发过程中尽量多次 Review ，既不是最后提测统一做一次，也不是每天做。只是这个结果我仍然悲观。

求助

不知道大家有没有遇到这种关于工程实践的分歧争论的情况，有没有什么良好的解决方案呀。自己默默 emo 中

[coolzjy]

你的想法是理想，小伙伴的想法是现实。

[zzfer]

别的不知道，互相 Review 效果挺差的。除非有一套人人都认可的标准且你们所有人都能认真执行。

[liuzhaowei55]

提高单元测试覆盖率吧，cr 在我看来大部份时间只能找出来一些语言层面的问题，对业务上的细节覆盖不到，除非这些业务细节大家都很熟悉。

[maichael]

在这方面来说没有一步到位的方法，总是先定个框架，然后再慢慢根据真实情况调整。

Code Reivew 最重要的是定标准，不然的话总会在 Review 过程中产生分歧，导致时间都浪费在这上面。

另外，说服其实不难； Review 一两百行的更改和 Review 一两万行的更改，你更愿意 Review 哪一个。更不要说后者这种长时间段的改动，往往写代码那个人都会忘记很多其中的细节，Review 的过程中有很多时间都会浪费在这上面。

[niceyuri]

@coolzjy 可能之前忘了补充前提，我的想法就是我之前的现实

[niceyuri]

@maichael 好办法！

[niceyuri]

@zzfer 一针见血。所以我们 Review 是基于公司的编码规范来的，同时也鼓励讨论最佳实践。

[iyaozhen]

个人认为 CR 还是非常有用

我是 QA ，RD 的代码我都会 CR 。
我对业务还是相对熟悉（这个前提很重要），CR 不是发现语法问题，这些静态代码检查就行了，主要是发现业务逻辑问题，还有实现的问题，实现不能得过且过，还是能发现很多问题的
还有个作用就是促进相互了解，两个人开发模块，可能都重复造轮子了，互通一下比较好。还有就是你以后也会开发别人的模块，相互了解团队人员也能相互 backup
其余还有个好处就是大家写代码会多想一点，把代码写好。我们团队之前就遇到一个模块的代码 review 了一星期还没合进去，有了这次经历后续那个 rd 写的代码质量就好很多了。（当然这个看人，说不定别人还恨你）

最后频率问题
「每天提交代码时互相 Review 一下，人也轻松」你这样不太好，代码不是按行或者按天分的，今天写的逻辑都不完整，而且每天提交也不是个好习惯。应该是能运行的一个子功能提交一次（ bugfix 除外），基于此 review 。提测前可以再拉上 QA 一起 review 下

[xuboying]

OP 提的 Review 是已经去除 LINT 等机器能自动扫描以后，单纯的人工 review 部分么？

[Rwing]

尽量把 codereview 放在 pull request 里，也就是每次的 pr 必须要 review 才能 merge

[MiniGhost]

你的观点是正确的，少量多次肯定是好的

我之前遇到过一些同事，一口气提一两千行代码，这是指望 Reviewer 是个神仙吗？
错误的习惯就应该就改正，而不是迁就他

还有就是，这种事情给我的启发跟教训就是，如果不是很好的团队，就不要搞民主，搞专政搞一言堂。

先把自己的方案推下去，日常多留心一下大家的执行情况，团队里面人员有执行不到位的即时纠正，之后再看情况了解了解大家的反馈，是否要做一些调整

[yzbythesea]

无法理解你的看法。确保代码质量不应该是依赖单元测试，整合测试，Canary 测试这些客观手段进行吗？第一过度依赖 CR 本身就不靠谱；第二 CR 质量和 CR 大小没有关联；第三无效的规章制度定得太多，会影响人家工作舒适感（就和打卡上班一样）

[niceyuri]

@xuboying 是的，规范、缺陷、依赖项漏洞都有静态扫描工具

[niceyuri]

@Rwing 我们用了 Gerrit ，只是说通过的权限放得很松，不想太过限制大家。大家一般就不看代码直接点了

[niceyuri]

@iyaozhen 感谢~ 拓展了新的视野

[longgediyi999]

让摸鱼的小伙伴无所遁形 一天了 啥也没写

[RiceNoodle]

团队一直在做 Code Review ，说一点我觉得好的方式
1. 功能完整再去做 review ，太零碎的 PR 进行 code review ，很难找到一些逻辑性或者结构性的问题。
2. 最好有个主程的角色，或者某个业务主程的角色，由他决定是否能够 approve ，允许 merge 。一票否决，这样能够保持某个功能模块，有统一的标准。

[smilenceX]

@yzbythesea 非常赞同。

[shanghai1998]

我们以前是（以前是）：每天早上站会，团队成员可以选择 今天 review 或者后面，但是每个功能总会 review 一遍，review 的时候可以看写的有没有问题或者漏洞，有没有更好的写法等，每次 review 都是一步小成长

[niceyuri]

@shanghai1998 这个办法应该团队小伙伴也能接受~