大家所在的公司都做 CodeReview 么？

[peterswan]

我现在所经历的公司，还没有做 CodeReview 的，也许公司太小的原因，但是我觉得 CodeReview 是非常有效的方式，能提高代码可控性和质量，还能促进技术交流。不知道你们经历的公司有 CodeReview 么，有的话可以分享一下感受，和如何进行 CodeReview 比较好。

[anonymous0user]

小公司比大公司更容易 CodeReview ……

[peterswan]

@anonymous0user 为啥我经历的小公司也没有 CodeReview，感觉有的公司是因为项目赶，有的根本就没有这个意识...

[F281M6Dh8DXpD1g2]

做数据库的，不做 code review 就是找死

[peterswan]

@liprais 数据库应该做 CodeReview 是必须的了，这个弄不好就从开发到跑路了。

[NCry]

我们公司就两个开发。。。

[peterswan]

@NCry。。。那你们俩启不是公司的核心技术了

[NCry]

@peterswan 公司业务比较简单，对于技术要求不高。实际上我感觉我在慢性自杀，但是又找不到方法跳出来，目前只能呆着。

[yuanrenxue]

@NCry 一个写代码，一个补漏 哇咔咔

[zclHIT]

做啊，每个版本快结束的时候改 pmd, checkstyle, codedex, simian 改到想吐，不过代码冲刺的时候 CodeReviewer 就直接点 accept，从来不看。。。

[enenaaa]

现在的这家不但做交叉 review，入库还要审批。 重构优化代码有时候还找不到理由发申请。

[xiaket]

找人过 PR 好烦... - .-

[kaito]

在公司推行 Code Review 的话需要看看周围同事是不是有那种意愿

如果是平级推进的话，看看同事的技术和代码风格是否 ok，可以的话找他讨论一下有没有意愿，有些程序员很无所谓的，觉得写「好代码」太浪费时间了，review 就是 挑刺的过程，自尊心太强的会非常抵触。

推行这些的前提需要比较好的同事关系和工作素质，如果都不错的话可以尝试推进一下

[peterswan]

@zclHIT 我觉得不错，对于代码质量和自己习惯都有提高，还可以对于优化进行讨论，不过代码冲刺的时候我觉得可以预留这部分时间，太急的情况只能例外了。

[hasbug]

上一家我去的时候给我说会，我害怕我的面条式代码会有危险，待了一段，没有的事。。。

[peterswan]

@kaito 恩恩，我觉得可以和同事商量一下，对于一些代码风格问题可以统一定一个规范，Review 过程也是提高的过程。

[peterswan]

@xiaket 等待 pr 的心理就像等待女朋友消息的心理，哈哈。

[peterswan]

@enenaaa 我觉得领导要是有技术热情，应该会喜欢优化的，重构要看付出的代价了。

[peterswan]

@hasbug 为啥，难道审批的人不在乎都给过。。。

[crayygy]

我们 team 里面是必须过的，而且至少有两个人，一个是需要是本模块的，另一个可以是其他的，而且 arch manager 都会查最近的 commit 有没有按照规定来完成

[mikuazusa]

持续集成流程强制必须过 CR

[winterfell30]

提代码的时候必须要有该模块的负责人打分，但是感觉大家也不怎么看随手就给过了

[zclHIT]

@peterswan 每次都能把一个月的需求压缩到一周然后搞冲刺，扯皮 2 周，冲刺 1 周，测 1 周

[peterswan]

@crayygy 你们这个流程可以的，避免了哪些乱七八糟的提交和代码。

[poorcai]

我们部门每周一次

[klren0312]

讲真的，做外包本来就很敢，恨不得早点写完，没人愿意看代码

[d18]

大公司也有代码质量很差的，只要最后能跑起来就行，比如鹅厂。

[wobushizhangsan]

今年做了个项目别说 review，连测试都没了。开发，上线，生产问题，补丁。

[wuzhizhan]

今天需求，明天上线

[dangluren]

哇，和楼主很像，目前的公司也不做，之前经历过的都做，虽然有时候有的自尊会受到打击，但感觉是非常有效的方式，由于不做，目前的项目是天天出问题。向领导反馈过，领导貌似自己都不想做

[peterswan]

@dangluren 恩恩 这个就像 12 楼所说的，要大部分人都有这个意识，尤其是技术领导或者团队领导，否则会趋于形式化起不到作用，现在我们也没有这个意识，大多数人更在乎速度，对于质量都想着以后优化，但是以后永远在以后。

[peterswan]

@d18。。。没去过大公司还，不过我觉得大公司推行这个应该更容易吧，资源充足，技术大牛也多，不过这个还是要看领导了

[monkeylyf]

长远来看不做 code review 就是自杀行为。
code review

[monkeylyf]

从中长远来没有 code review 看就是自杀行为。
了解同事的技术水平，了解同事在做什么，肉眼抓 bug，等等等。 因为要急着上线不审代码的先不说技术水平如何， 工程水平肯定有提高空间。

[HuHui]

回武汉后就没做过了。

[hiluluke]

先发 issue,然后技术讨论，然后再发 pr。pr 再 review 一遍。PR 过大，需要拆分。。。

[imdupeng]

没有 codereview，不过老大总喜欢改别人的代码，每次他的意见都很中肯，但是你改了要调试好啊，每次改了出问题，还得我去调试好。
一方面催进度，一方面跑来改我代码找麻烦。。能不能等逻辑跑完了再来优化呀？搞得我反复在原来的代码上做工。

[mhtt]

其实对没有足够的 code review 经验，而去做 code review 是件挺难受的事情

[coolhubery]

我部门是做数据库执行引擎的，C++ 14。CodeReview 到了“变态”的地步。。。
所有 change 都必须尽量的小，必须写 Unit Test，代码覆盖率 100%。一个 change 从开始 push 到最终 merge 最少怎么着也得 5 到 6 轮，当然国外的大牛同事基本上 2/3 个 patch 就 merge 了。
从错别字，注释的准确性，英语表达是否合适，代码可重用，设计角度是否合适，性能是否有影响，Unit Test 是否真得测到了该有的功能，是否应用了 C++ 14 的标准等诸多方面进行考量。。。
这样做的好处是作为非常核心的一个组件，模块非常稳定，新增代码量很大，bug 却非常少。
虽然作为个人来讲，初期自己的修改进展很慢，要被 challenge 很多次，但是一旦熟悉了也就很快了。
不管是那个公司，核心的组件必然要经过极其严格的 Review，否则后期的成本会非常高。

[nicevar]

配置 SonarQube，减少 code review 工作量，同是提升代码质量，还能让人的强迫症变得更严重

[Just1n]

我们每个 checkin 都必须有 code review。
而且，如果修改了不属于自己组的代码，会自动触发一个流程让其他组的人介入进行 review。

[peterswan]

@monkeylyf 同意啊，见了一些项目因为技术债实在开发不下去的项目。。。

[czk1997]

能跑就行，出 Bug 再说……测试全靠脸（能跑就行），服务器部署全随缘..

[peterswan]

@hiluluke 这个工作流程有点像 github，你们用的啥平台，github ？？ 我也想用这个流程试试

[peterswan]

@imdupeng 下次不要让他直接该代码，让他提 pr 或者开个分支，做好了之后你来 Review，有问题让他改好了在合并，不过这好像是你的老大。。。

[peterswan]

@coolhubery 这个流程有很多值得学习的地方啊，能提供这样严格的 Review 肯定公司也特别棒了，谢谢分享

[lsyAndroid]

没有，全靠自觉，本身就是赶项目的，没有时间搞

[peterswan]

@nicevar 用自动化工具是会加快这个过程，不过我感觉每次 pr 应该粒度尽可能的小，Code Review 压力就不会那么大了，而且可控性更高，如果太大的 pr，直接让他拆分后再 Review。

[peterswan]

@Just1n 恩恩 强制的 Review 才能保证代码可控，不过我觉得为什么要让其他组的人来 review 呢，毕竟其他组的肯定不熟悉业务，找同组的人会不会更节省时间。

[66beta]

本组的人，没人负责一个模块，也只能 review 语言本身了，无法察觉业务漏洞

[peterswan]

@czk1997
@IsyAndroid
我觉得这样做会加大技术债，现在赶出来的时间以后会加倍付出对于项目来说。当然要看老大有没有这个意识。。。

[A555]

形式大于内容

[peterswan]

@mhtt 刚开始要是没有经验，可以大家一起开个会学习一下 Review 嘛，以后应用到项目中就会越来越熟练了。

[peterswan]

@66beta 对于语言和代码优化相关的 Review 也会提升代码质量，更关键能在 Review 过程中提升自己，既可以挑别人代码的不足也可以学习别人代码的优点。

[hiluluke]

@peterswan 就是用的 github 企业版。

[natscat]

做啊 代码功能 实现方式 都做

[monkeylyf]

@peterswan 我亲身经历过没有代码审核，快糙猛急着上线，等代码库到一定体量一个没更新一个版本都是提心吊胆，开发人员身心俱疲，最后项目烂了，公司黄了。
话说回来，我现在所待的地方，是有代码审核的，同样老板喜欢催上线，大佬们都相互给个 LGTM 都不仔细看。现在代码体量已经很大了，基本两天一个中性 bug 爆炸，一周一个大 bug 核爆。
流程是用来约束自觉性的，但是完全没有自觉性，有再多流程也没用。

[loveCoding]

组内在使用 gerrit 做强制 code review 还算比较好用

[peterswan]

@monkeylyf 恩恩，这个要看有没有一个厉害的技术管理去领导做这个 Review 事情，对 Review 质量进行把关，如果只是应付形式简直就是浪费时间了，还有就是对于 Code Review 可以有一定的奖励制度调用积极性。

[RorschachZZZ]

@monkeylyf 中等或者严重的 bug，你们那不复盘下、讨论下吗

[zhaoxinz]

在就职过的公司中，只有印象笔记有融入在日常工作中的标准流程 Codereview，在 commit 之后，通过命令行指定一个人为你 Codereview

[hoichallenger]

我们公司有 Code Review, 不过正打算用 Pair Programming + Mob Programming 取代 Code Review

[monkeylyf]

@RorschachZZZ 讨论的。但都是基于表面或者 bug 本身的讨论，深层次原因应该大家都知道但是不会拿出来说罢了。

[xufeng]

@NCry 简单的事情可以创新做，《并非喊口号》

[peterswan]

@hoichallenger 刚刚了解了 Pair Programming 和 Mob Programming，真是个不错的想法，公司的技术管理也很不错啊。

[peterswan]

@loveCoding 准备搭建一发推荐给团队用。。。

[agagega]

你们在 review 时有些特别特别小的改动也要新增提交吗？有时候想 commit amend 然后 force push，但是又觉得不太好。看来还是 git 太自由了，用 hg 就没有这种强迫症。

[fanqianger]

@coolhubery 你们什么公司。

[peterswan]

@agagega 应该是按照功能细分粒度的，我觉得尽可能小的好。还有远程提交信息不是不应该修改嘛，除非只有自己提交分支。而且我觉得应该过了 Review 再入库。

[nanyang24]

很幸运的是，所在的公司技术 leader 强制了这项规定，合并代码前必须经过任意两个同事的 CodeReview。
我觉得好处是项目代码健壮性得到了一定的保证，对个人的话是学习认识不同风格和 level 的代码，还能熟悉别人做的需求，对整体项目理解有帮助。

[qiaobeier]

@coolhubery sap 核心部门的啊，运气真不错，我前同事分去做业务据说非常废人。。。

[agagega]

@peterswan 嗯，我说的就是自己的 PR 里面

[coolhubery]

@fanqianger SAP...

[coolhubery]

@qiaobeier 不了解产品，SAP 的数据库平台技术还是相当不错的。

[ymj123]

@zclHIT findbugs 不跑？

[fanqianger]

@coolhubery 这厉害的啊，hana 挺牛逼的。你是在德国吗

[liyuhang]

我觉得应该先 Review 一下你的标题

[jiangjz]

不做，所以有的代码维护起来让人想离职😂😂

[jaaazzz]

中等规模公司，做个屁

[micean]

作为管理人员想知道你们有多少时间花在 code review 上，我每天都忙得要死
现在只能了解下面人的代码水平，交流一些好的习惯，从编码规范上要求他们自觉

[dosmlp]

没。。。。。。时。。。。。。间。。。。。。

[lxerxa]

团队小的时候还做，后来慢慢放弃了。。。

[wuhanchu]

有 review 都是好公司啊

[hasbug]

@peterswan 因为项目已经不受项目经理控制，一个老员工总把自己当回事，什么都他说了算，项目经理心累，直接不管了

[peterswan]

@liyuhang 欢迎提 pr

[shijingshijing]

@coolhubery 如果养成习惯了就真不错，大家共同进步，code review 还有一些计划类的 paperwork 其实从长远看都是必不可少的。如果为了赶工期某一个人开始写 smelly code，而且不加制止就 merge 进来，最终导致的是整个 project 散发臭味。就跟加班一样。

ps：比较好奇你们的覆盖率达到 100%，是指需求覆盖率还是语句覆盖率？如果是严格按照 MCDC 做语句覆盖，每个条件都 cover 到，那 100%就厉害了。

[peterswan]

@jiangjz 同感，有的代码看着让人头皮发麻，经过了 n 个人的修改，修改成本很高，只能重构。

[UIXX]

有 CodeReview 要两点：
1、领导强制力
2、员工自觉性
团队要有能人，员工要会进取、肯学习。其他都是扯淡。

[coolhubery]

@shijingshijing 语句 100%覆盖。除了极少部分遗留代码无法实现 100%的情况外，新增代码必须语句 100%，否则 Gerrit CheckBot 静态检查都过不了。

[coolhubery]

@fanqianger 西安...

[peterswan]

@micean 就我的经历来看，靠自觉解决不了工程中出现 smelly code，只要有这种代码，就很有可能会让项目代码维护成本增高，而且如果维护的人写的代码继续糟糕下去，会使的项目走向慢性死亡，这些时间还不如拿出来放到开发的时候去 Review，其实是节省的整体项目的时间，管理人员更应该有这个意识。

[shijingshijing]

@coolhubery 那这个成本很高了，我只在某些 safty critical 的软件上看到过类似甚至更严的方法，比如 pacemaker 产品不仅对覆盖率有要求，而且对实时性有非常高的要求，必须在指定时间内完成指定任务。

你们测试是自己做还是外包给三哥？包括 test plan，test case，test report 都是怎么做的？

[Rico]

有。且代码评审作为升等的一个参数。

[thinkmore]

每次提交代码都要 code review.

好处：学习别人写的好的代码，让别人指出自己写得不合理的地方或者造成歧义的地方。

坏处：codereview 有时候时间太长，无法快速合并代码影响开发效率。

[coolhubery]

@shijingshijing SAP HANA 内存数据库执行引擎。
因为是全新的执行引擎去替代老的引擎，所以大部分的端到端测试，性能测试，压力测试等都可以重用。自己的 change 提交的话是需要自己写 unit test 的，一些本引擎特有的特性需要自己额外写 E2E 测试。

[xoxo419]

人少 活多 事赶

[wujiyu115]

阶段性 review,一个开发周期结束,上线之前 review 一下

[woofee]

@NCry 结对编程

[micean]

@peterswan 本来也是半路接手的项目，项目本身的设计就有问题，从去年开始就跟上头不止一次提过要重构整个设计，以免项目越来越膨胀的时候彻底改不动了。然而一点卵用都没有，1 版本还在测试，2 版本的需求就过来了，3 版本的需求就开始规划了，设计思想基本没有，有点无力回天的感觉了

[peterswan]

@micean 对于这种项目，上头的人如果只想着眼前开发的速度，不考虑可维护性，如果你本身也没这么大的权利去实施重构和审查，我觉得这个项目已经走向凉凉的道路了。如果能够实施，复盘重构是必须的了，重构加入代码审查，人力充足的话老版本的迭代和重构版本同步进行，员工心里写代码也会开心提高效率。

[mingyun]

那么问题来了，有什么好的 code review 工具